fix: text changes in business central configuration#1089
fix: text changes in business central configuration#1089JustARatherRidiculouslyLongUsername merged 3 commits intobusiness-central-betafrom
Conversation
WalkthroughThe changes in this pull request involve updates to the HTML templates of three components: Changes
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (1)
102-104: LGTM! Consider enhancing the warning message.The warning message about Spend Date usage is well-placed and the conditional logic is correct. However, the message could be more actionable.
Consider enhancing the warning message to guide users toward the preferred approach:
- <span>Using the Spend Date may lead to multiple single line<br>item entries.</span> + <span>Using the Spend Date may lead to multiple single line<br>item entries. Consider using Report Date for consolidated entries.</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html(3 hunks)src/app/integrations/business-central/business-central-shared/business-central-import-settings/business-central-import-settings.component.html(2 hunks)src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html(1 hunks)
🔇 Additional comments (5)
src/app/integrations/business-central/business-central-shared/business-central-import-settings/business-central-import-settings.component.html (2)
29-29: LGTM! Enhanced clarity in Vendor import description.
The revised text maintains consistent structure with the previous change and clearly explains the purpose of vendor imports in the context of merchant field usage.
Let's verify consistent vendor/merchant terminology:
✅ Verification successful
Terminology consistency verified across Business Central integration
The verification shows consistent usage of vendor/merchant terminology across the Business Central integration:
- Configuration property uses
import_vendors_as_merchants - Form control uses
importVendorAsMerchant - UI labels consistently explain "Import Vendor as Merchant" concept
- The tooltip text (subject of review) aligns with this established pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent vendor/merchant terminology
# Look for vendor/merchant references in related components
rg -g '*.{html,ts}' -i "vendor.*merchant" src/app/integrations/business-central/
Length of output: 1023
19-19: LGTM! Improved clarity in Chart of Accounts import description.
The updated text provides clearer guidance to users about where imported accounts will appear and how they can be used, which aligns well with the PR's objective of improving text clarity.
Let's verify consistent terminology usage across related components:
✅ Verification successful
Let me gather more information to verify the terminology consistency by checking for any variations of category-related terms in the business central components.
Terminology is consistent across Business Central components
The search results confirm that:
- The term "Category" is consistently used across the Business Central integration
- The UI pattern of using dropdowns for selection fields is consistently implemented
- The terminology aligns with the mapping and dashboard components where "Category" is used as a field reference
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent terminology usage across components
# Look for 'Category' field references in related components
rg -g '*.{html,ts}' -i "category.*field" src/app/integrations/business-central/
Length of output: 441
Script:
#!/bin/bash
# Search for variations of category terms in business central components
rg -g '*.{html,ts}' -i "categor" src/app/integrations/business-central/
# Also check for dropdown references to ensure UI consistency
rg -g '*.{html,ts}' -i "dropdown" src/app/integrations/business-central/
Length of output: 5521
src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html (3)
76-76: LGTM! Clear mapping terminology.
The label now explicitly mentions "Vendors" as the destination entity in Dynamics 365 Business Central, which improves clarity about the mapping relationship.
255-255: LGTM! Improved state descriptions for corporate card expenses.
The sub-label now accurately reflects the available states:
- "Approved" instead of "Payment processing"
- "Closed" instead of "Paid"
This provides clearer guidance about when expenses can be exported.
125-125: Verify state terminology consistency across the application.
The change from "Paid" to "Closed" for reimbursable expenses improves accuracy, but we should ensure this terminology is consistent across the application.
✅ Verification successful
Based on the search results, I can see a consistent pattern across different integrations (Business Central, QBO, Xero, NetSuite, etc.) where the terminology has been updated from "Paid" to "Closed" when the is_simplify_report_closure_enabled flag is enabled. This change is consistently applied across the codebase with proper conditionals.
For example:
- In QBD:
label: this.is_simplify_report_closure_enabled ? 'Closed' : 'Paid' - In Intacct:
label: this.is_simplify_report_closure_enabled ? 'Closed' : 'Paid' - In Business Central, QBO, and other integrations: The terminology "Closed" is used in the export settings when referring to settled payments
The subLabels across different components also consistently use this terminology:
- "when the payment has been settled (Closed)"
- "awaiting closure after approval (Processing) or when the payment has been settled (Closed)"
The terminology change appears to be consistently implemented across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of expense states across the codebase
# Look for any inconsistencies in state terminology
# Search for variations of expense states
echo "Checking for expense state terminology consistency..."
rg -i "paid|closed|payment processing|approved" --type html --type ts
Length of output: 35032
|
JustARatherRidiculouslyLongUsername
requested review from
DhaaraniCIT and
ashwin1111
* fix: remove export link column from export logs * fix: show a 'Beta' badge for business central in landing (#1090) * fix: show a 'Beta' badge for business central in landing * fix: add orgs to hide the 'Beta' badge from * fix: text changes in business central configuration (#1089) * fix: export settings content changes * fix: hide spend date help message for group by expenses * fix: update import settings sublabels
3 participants
Clickup
https://app.clickup.com/t/86cx5t0y4
Summary by CodeRabbit
New Features
Bug Fixes