Skip to content

[29.0] - Bug 650939 - [Expense Agent] Deleting an itemized Expense Category shows misleading missing-subcategories confirmation - #11707

Open
v-rohangarg20 wants to merge 1 commit into
releases/29.0from
bugs/Bug-650939-Deleting-itemized-Exp-Cat-shows-misleading-missing-subcategories
Open

v-rohangarg20 wants to merge 1 commit into
releases/29.0from
bugs/Bug-650939-Deleting-itemized-Exp-Cat-shows-misleading-missing-subcategories

Conversation

@v-rohangarg20

@v-rohangarg20 v-rohangarg20 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

@v-rohangarg20
v-rohangarg20 requested a review from a team September 22, 2026 06:30
@v-rohangarg20
v-rohangarg20 requested a review from a team as a code owner September 22, 2026 06:30
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area labels Sep 22, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Sep 22, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

The change skips the missing-subcategory close confirmation while an itemized expense category is being deleted, so the category can be deleted after its subcategories are cascade-deleted. It also applies the same pattern to the expense-user close confirmation.

The direction matches the reported scenario: the warning should protect retained setup records, not a record that is being deleted. The delete flag covers the expected successful delete path, but it is set before all delete paths are known to continue, which leaves one non-blocking edge case.

Problem-solution fit

Fit: Strong

The fix addresses the reported category deletion flow and keeps the existing validation for normal page close. The extra expense-user change follows the same UX pattern, but the new guard state should be made safe when a delete attempt is canceled or rejected.

Suggestions

S1 (🟠 Moderate): Deleting flag can survive canceled deletes
Set the deleting flag only after the delete will continue, and clear it if the delete is canceled. Today the flag is set before all delete checks have succeeded, so a canceled or failed delete can leave the page open with the close-time warning disabled. This affects the same guard pattern added on both pages.

Risk assessment and necessity

Risk: Low to moderate. The change is limited to setup-page delete and close prompts in Expense Agent, not posting, amounts, or persisted cascade logic. The main regression surface is suppressing setup warnings after a delete attempt that does not actually delete the record.

Necessity: The category change is needed because deletion can legitimately remove the related subcategories before the page closes, making the old confirmation misleading. The scope is small, but the delete guard should track only a delete that really proceeds.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11707 round=1 by=alexei-dobriansky at=2026-09-22T07:13:35Z lastSha=7a392cfb9521b9eaab70c69ef6b0996511a36fdb reviewKey=60f48d66f92f1ae011fb7f0d5c0198adb96981e8ea3d0f398813b48b062e4e32 suggestions=S1@6029a756

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants