[29.x] - Bug 650481[Expense Agent] Closed travel requests are editable and are missing approver info. - #11593
v-rohangarg20 wants to merge 9 commits into
Conversation
Good Sense Reviewer - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis change makes travel-request fields editable only while the request is open, stores the requested-for name, and records complete approval details when a request is approved automatically. It also resolves the displayed user name from the user record for normal approval and rejection. The status guards and approval metadata updates match the requested behavior. However, the new derived name can be edited independently, and unused page state currently causes the app builds to fail. Problem-solution fitFit: Partial The change covers the travel-request editing and approval-recording paths, but the editable derived name can become inconsistent with the selected expense user. The implementation also needs a small cleanup before it builds without new warnings. SuggestionsS1 (🟠 Moderate): Verify the new automatic approval metadata S2 (🟠 Moderate): Keep the derived user name read-only S3 (🟠 Moderate): Remove or use the unused page state Risk assessment and necessityRisk: The approval changes affect audit metadata rather than posting amounts. The main regression risks are an incorrect displayed person, missing approver details after automatic approval, and the current build failure caused by the unused variable. Necessity: Open travel requests need editable planning details, and approved requests need an accurate approver identity. The scope is reasonable, but the derived-name consistency and build warning should be corrected.
|
Allow editing the approver on Travel Request cards and preserve the approver update behavior across the Expense Agent and Spend Request objects. (cherry picked from commit abe7b2a)
9357d1c to
97578a2
Compare
Good Sense Reviewer - Round 2Recommendation: Accept with SuggestionsWhat this PR doesThis update removes the unused page state that was introduced in the first round while keeping the travel-request editability, requested-for name, and disabled-agent auto-approval changes. The build-warning part of the prior review is addressed, but the derived requested-for name is still editable and the new approval metadata still has no focused test coverage. Problem-solution fitFit: Partial The change now fits the requested behavior better because the unused page state is gone. It is still partial because the stored requested-for name can still be changed independently from the selected expense user, and the new approval metadata is not covered by a regression test. Status of previous suggestions
New observations (commits since round 1)None - the latest change only removes the unused page state from the first review. Risk assessment and necessityRisk: The build-warning risk from the unused variable is gone. The remaining risk is data consistency: users can still edit a name that is derived from another field, and the approval metadata can regress because it is not asserted by tests. Necessity: The edit guards and approval metadata are needed for closed travel requests and auditability. The scope is still reasonable, but the derived-name consistency and focused test coverage should be completed.
|
…age" This reverts commit 52884eb.
Good Sense Reviewer - Round 3Recommendation: Accept with SuggestionsWhat this PR doesThis round makes travel request fields editable only while the request is open, records approver identity and time for automatic approval, stores the requested-for name, and adds focused tests for approval metadata and editability behavior. Status of previous suggestions
New observations (commits since round 2)S4 (🟠 Moderate): Keep the derived requester name read-only Risk assessment and necessityRisk: Approval audit and closed-request edit guards are covered better now. The remaining risk is data consistency for a derived display name. Necessity: The change is needed so closed travel requests cannot be edited and automatic approvals carry audit metadata. The derived requester name should not remain directly editable.
|
Good Sense Reviewer - Round 4Recommendation: Accept with SuggestionsWhat this PR doesThis round adds the remaining editability guards for travel request fields while keeping the automatic approval metadata and requester-name updates from earlier rounds. The closed-request edit path is better covered now: purpose, actual dates, and the card number field follow the open-status guard, and automatic approval still stores user and time metadata. The remaining concern is the derived requester name field. Status of previous suggestions
New observations (commits since round 3)None - the new commits add more read-only guards, but they do not fully address S4. Risk assessment and necessityRisk: The main editability and automatic-approval audit paths are now covered by code and tests. The remaining risk is data consistency: a derived requester name can be changed independently from the requester number while the request is open. Necessity: The change is needed so non-open travel requests cannot be edited and automatically approved requests carry approver metadata. The scope is appropriate for the bug, with one non-blocking consistency issue left.
|
Fixes AB#650481
Problem
Closed travel requests can still be edited, and automatically approved travel requests do not record approver information. The requested-for name is also not shown on the travel request card.
Changes