-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: intacct code prepend fixes #965
Conversation
WalkthroughThe pull request introduces several changes across multiple components in the Intacct integration. Key modifications include the addition of a new input property Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.ts (1)
31-38
: LGTM! Consider using a regular expression literal.The new regular expression pattern correctly handles field names containing special characters, enhancing the robustness of the transformation logic.
Based on the static analysis hint, consider using a regular expression literal instead of the
RegExp
constructor for improved performance and maintainability.Apply this diff to use a regular expression literal:
-const pattern = new RegExp('[^a-zA-Z\d\s:]'); +const pattern = /[^a-zA-Z\d\s:]/;Tools
Biome
[error] 31-31: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.html (1 hunks)
- src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.ts (2 hunks)
- src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-base-mapping/intacct-base-mapping.component.ts (1 hunks)
- src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts (5 hunks)
- src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.html (1 hunks)
- src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.ts (1 hunks)
Additional context used
Biome
src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.ts
[error] 31-31: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
Additional comments not posted (9)
src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.html (1)
44-44
: LGTM!The addition of the
[importCodeFields]
input property to theapp-dashboard-error-section
component is a good enhancement to handle specific code fields during the import process.To ensure the correctness of this change, please verify that the
acceptedCodeField
variable is correctly populated with the expected set of code fields. You can add a console log or a debugger statement to inspect the value ofacceptedCodeField
during runtime.src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.html (1)
7-7
: Verify theapp-card-mapping-header
component handles thesourceType
property correctly.The change in property binding from
sourceField
tosourceType
looks good. However, please ensure that theapp-card-mapping-header
component is equipped to handle thesourceType
property and that it displays the correct data in the header.Run the following script to verify the usage of the
sourceType
property in theapp-card-mapping-header
component:src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-base-mapping/intacct-base-mapping.component.ts (1)
41-41
: LGTM! Verify the impact of this change on the codebase.The addition of
IntacctCategoryDestination.EXPENSE_TYPE
to theacceptedCodeField
array is a valid change that expands the component's capability to handle expense-related data. This aligns with the overall purpose of the component.Please ensure that this change is properly handled in other parts of the codebase that interact with this component. You can use the following script to search for potential impact areas:
Verification successful
Verification complete: Change is consistent and properly implemented.
The addition of
IntacctCategoryDestination.EXPENSE_TYPE
to theacceptedCodeField
array has been consistently applied across the codebase. This change is present in both theIntacctBaseMappingComponent
and theIntacctDashboardComponent
, ensuring consistency in the handling of expense types.The
IntacctBaseMappingComponent
is properly integrated into the module structure, routing, and has a corresponding test file. This indicates that the change has been implemented correctly and is well-supported within the existing architecture.Based on this analysis, no further manual verification is required. The change appears to be correctly implemented and doesn't have any negative impact on the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of `acceptedCodeField` in the codebase. # Test: Search for the variable usage. Expect: Occurrences in relevant files. rg --type typescript -A 5 $'acceptedCodeField'Length of output: 80
Script:
#!/bin/bash # Description: Search for usage of `acceptedCodeField` in TypeScript files # Approach 1: Using fd to find TypeScript files and xargs to run rg echo "Approach 1: Using fd and xargs" fd -e ts -e tsx | xargs rg -n -A 5 'acceptedCodeField' # Approach 2: Using rg with file extension patterns echo "Approach 2: Using rg with file extension patterns" rg -n -A 5 'acceptedCodeField' --glob '*.{ts,tsx}' # Additional search: Look for files that import or use IntacctBaseMapping echo "Additional search: Files importing or using IntacctBaseMapping" rg -n -A 5 'IntacctBaseMapping' --glob '*.{ts,tsx}'Length of output: 10337
src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.ts (2)
2-4
: LGTM!The changes in the import statements are appropriate and align with the component's functionality.
89-90
: Verify the usage of theacceptedCodeField
property.The
acceptedCodeField
property is added to the component, but it is not used anywhere else in the component's logic. Please ensure that this property is properly utilized or remove it if it is no longer needed to avoid confusion and maintain code clarity.src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts (4)
8-8
: LGTM!The newly imported enums will help make the code more readable and maintainable by using named constants.
301-301
: LGTM!The changes in the
addImportCodeField
method look good:
- Mapping
GL_ACCOUNT
toACCOUNT
standardizes the field name.- Validating against
acceptedImportCodeField
ensures only allowed fields are added.- Correctly handles adding/removing the form group based on
event.checked
.
555-558
: LGTM!The changes in the
initializeForm
method look good:
- Mapping
GL_ACCOUNT
toACCOUNT
ensures the correct field is used in subsequent logic.- The multiple checks before adding the import code field ensure it's added only when necessary and with proper configuration.
Also applies to: 560-560
642-642
: LGTM!Using the
reimbursable_expenses_object
andcorporate_credit_card_expenses_object
configurations to determine theintacctCategoryDestination
is a good approach. It ensures the correct category is used during import based on the expense object setup.
@@ -28,11 +28,14 @@ export class MappingCardHeaderComponent implements OnInit { | |||
destinationField = new SnakeCaseToSpaceCasePipe().transform(destinationField).toLowerCase(); | |||
const lastChar = destinationField.slice(-1).toLowerCase(); | |||
const lastTwoChars = destinationField.slice(-2).toLowerCase(); | |||
const pattern = new RegExp('[^a-zA-Z\d\s:]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are we doing here?
@@ -4,7 +4,7 @@ | |||
|
|||
<div> | |||
<div> | |||
<app-card-mapping-header *ngIf="mappings" (triggerAutoMapEmployee)="triggerAutoMapEmployees()" [sourceField]="sourceField" [showAutoMapEmployee]="showAutoMapEmployee" [mappingStats]="mappingStats" (triggerAutoMapEmployee)="triggerAutoMapEmployees"></app-card-mapping-header> | |||
<app-card-mapping-header *ngIf="mappings" (triggerAutoMapEmployee)="triggerAutoMapEmployees()" [sourceField]="sourceType" [showAutoMapEmployee]="showAutoMapEmployee" [mappingStats]="mappingStats" (triggerAutoMapEmployee)="triggerAutoMapEmployees()"></app-card-mapping-header> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
affects all app, please test mapping page for every integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, tested @ashwin1111 all are working fine
PR title must start with "fix:", "feat:", "chore:", "refactor", or "test:" (case-insensitive) |
PR description must contain a link to a ClickUp (case-insensitive) |
PR description must contain a link to a ClickUp (case-insensitive) |
intacct code prepend fixes
intacct code prepend fixes
Summary by CodeRabbit
New Features
Bug Fixes
Improvements