Test BCQuality code review main setup - #962
Open
aholstrup1 wants to merge 2 commits into
Open
Conversation
…to test-bcquality-code-review-main-setup
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
🟡 Changes recommended
Rate application ordering can omit taxable amounts, and fixed-asset lines can retain stale item-category data.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This test PR exercises BCQuality review setup through an Excise Taxes enhancement supporting item-category, ad-valorem, and hybrid rates.
Changes:
- Replaces the legacy rate model with a migrated, extensible rate table and page.
- Adds taxable-amount calculations and transaction logging.
- Updates permissions, test helpers, and calculation tests.
File summaries
| File | Description |
|---|---|
test/src/LibraryExciseTax.Codeunit.al |
Updates rate test helpers. |
test/src/ExciseTaxCalculationTests.Codeunit.al |
Uses the new rate table. |
test/src/ExciseCalculationTypeTests.Codeunit.al |
Tests new calculation modes. |
tableextension/ExciseTaxesTransLogExt.TableExt.al |
Adds logged rate fields. |
tableextension/ExciseJournalLineExt.TableExt.al |
Adds and calculates rate components. |
table/ExciseTaxType.Table.al |
Checks new rate dependencies. |
table/ExciseTaxRate.Table.al |
Defines the new rate model. |
table/ExciseTaxItemFARate.Table.al |
Obsoletes the legacy model. |
Permissions/ExciseTaxesRead.permissionset.al |
Adds read permissions. |
Permissions/ExciseTaxesObjects.permissionset.al |
Adds object permissions. |
Permissions/ExciseTaxesEdit.permissionset.al |
Adds edit permissions. |
pageextension/ExciseTaxTransLogExt.PageExt.al |
Displays logged calculation details. |
pageextension/ExciseJournalLineExt.PageExt.al |
Displays new journal fields. |
page/ExciseTaxTypes.Page.al |
Opens the new rates page. |
page/ExciseTaxTypeCard.Page.al |
Opens the new rates page. |
page/ExciseTaxRates.Page.al |
Configures new rates. |
page/ExciseTaxItemFARates.Page.al |
Obsoletes the legacy page. |
enum/ExciseCalculationType.Enum.al |
Defines calculation modes. |
codeunit/ExciseTaxUpgrade.Codeunit.al |
Migrates legacy rates. |
codeunit/ExciseTaxTransSubscriber.Codeunit.al |
Resolves and validates rates. |
codeunit/ExciseTaxCalculation.Codeunit.al |
Derives taxable amounts. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+290
to
+292
| if RequiresTaxableAmount(ExciseJnlLine) then | ||
| ExciseJnlLine.Validate("Excise Taxable Amount", GetTaxableAmountFromItemLedgerEntry(ItemLedgerEntry)); | ||
| OnAfterUpdateExciseJournalLineFromItemLedgerEntry(ExciseJnlLine, ItemLedgerEntry); |
|
|
||
| // The taxable value of an item ledger entry is taken from its value entries: the invoiced sales or purchase | ||
| // amount when the entry has one, otherwise the inventory cost. | ||
| local procedure GetTaxableAmountFromItemLedgerEntry(var ItemLedgerEntry: Record "Item Ledger Entry"): Decimal |
| ExciseJournalLine.Validate("Excise Unit of Measure Code", FixedAsset."Excise Unit of Measure Code"); | ||
| ExciseJournalLine.Validate("Quantity for Excise Tax", FixedAsset."Quantity for Excise Tax"); | ||
| ExciseJournalLine.Validate("Excise Duty", GetExciseDutyForSource(ExciseJournalLine."Excise Tax Type", ExciseJournalLine."Source Type", ExciseJournalLine."Source No.", ExciseJournalLine."Posting Date")); | ||
| ApplyExciseRate(ExciseJournalLine, ''); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test PR for validating whether default-branch Copilot code review setup steps can install the BCQuality plugin and make its marketplace skill available to the built-in reviewer.