Slice 580818: [Expense Management] - Role Centers updates 💸 - #10677
ViditGupta1277 wants to merge 6 commits into
Conversation
GitHub Actions
Failure
…res/Slice-580818-Expense-Management-Role-Centers-updates
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -62,5 +62,6 @@ permissionset 6907 "Expense Mgmt. Read" | |||
| tabledata "Expense VAT Specification" = R, | |||
| tabledata "Expense Report Line VAT Spec." = R, | |||
| tabledata "Expense Activity Log Entry" = R, | |||
| tabledata "Expense Activities Cue" = RIMD, | |||
There was a problem hiding this comment.
"Expense Mgmt. Read" (permissionset 6907) grants tabledata "Expense Activities Cue" = RIMD, while every other tabledata grant in this same read-only permission set is R only. This is an inconsistent over-grant: a permission set named and captioned "Expense Management - Read" should not hand out Insert/Modify/Delete rights on a table, especially since the cue table's own OnOpenPage trigger (Rec.Init/Rec.Insert) is what actually needs write access. Either move the row-initialization logic behind a call that runs under a more privileged/system context, or scope the write rights to the Edit permission set (which is already granted IMD on the same table) and keep Read at R.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
There was a problem hiding this comment.
it should be Read Only
|
Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4 |
Jesper Schulz-Wedde (JesperSchulz)
added
the
Team: Finance
Good Sense Reviewer - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis change adds an Expense Activities cue part to the Accountant, Accounts Payable Administrator, and Business Manager role centers. The cue table counts expense reports by status and adds drilldowns to the expense report, expense, and rule-violation lists. The status cues mostly map directly to the new flow fields, and the singleton cue record follows the usual role-center pattern. Two details should be tightened: the new part is always added to standard role centers without an Expense permission guard, and the policy-violated cue counts violation rows rather than expenses. Problem-solution fitFit: Partial The role centers receive the requested cues and drilldowns for the main report statuses. The fit is not complete because one cue name says it counts policy-violated expenses, but the implementation counts rule-violation records, which can be more than one per expense. SuggestionsS1 (🟠 Moderate): Hide the part without Expense permissions S2 (🟠 Moderate): Count violated expenses, not violations Risk assessment and necessityRisk: The change does not affect posting or ledger data, but it changes highly visible role centers. The main risks are role-center load or permission friction for users without Expense access, and misleading cue numbers for policy violations. Necessity: The requested role-center visibility is a reasonable feature increment for expense management. The scope is also right for a role-center update, but the permission and cue-semantics details should be corrected before relying on the new tiles.
|
…e-Centers-updates
Good Sense Reviewer - Round 2Recommendation: Accept with SuggestionsWhat this PR doesThis change adds Expense Activities cues to the standard finance role centers and wires drilldowns for expense reports, released expenses, and policy violations. The only new code-bearing change since round 1 renumbers the new cue page and cue table and extends the app object range; that change is consistent with the final net diff. The role-center wiring and object renumbering are low-risk metadata/UI changes. The earlier cue visibility and policy-violation count concerns are still present in the current diff. Status of previous suggestions
New observations (commits since round 1)None - the new commit only renumbers the new objects and updates the app object range. Risk assessment and necessityRisk: The remaining risk is UI accuracy and role-center friction, not posting or ledger integrity. Users can still see an Expense part on standard role centers without a page-level permission guard, and the policy-violated cue can still show violation records instead of distinct expenses. Necessity: The role-center visibility is a valid feature increment for expense management, and the new object numbers are necessary after the renumbering. The two earlier suggestions should still be considered before relying on these cues in preview.
|
|
The changed permission-set file is named Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4 |
Good Sense Reviewer - Round 3Recommendation: AcceptWhat this PR doesThis change adds an Expense Activities cue part to the accountant, accounts payable administrator, and business manager role centers. It shows expense report status counts, released expenses, and expenses with policy violations, with drilldowns to the matching lists. The changes since round 2 address the remaining issues: the cue part is now hidden unless the user can read Expense records, and the policy-violated cue now counts and opens expenses instead of violation rows. The added data classification for the cue key is also consistent with the singleton cue-table pattern. Status of previous suggestions
New observations (commits since round 2)None - the new commits address the previous suggestions and add cue-key data classification. Risk assessment and necessityRisk: The remaining risk is limited to role-center UI and cue accuracy. The change does not touch posting, approvals, ledger entries, or amount calculation, and the new permission guard reduces role-center friction for users without expense access. Necessity: The requested role-center cues are a coherent feature increment for expense management. The round 3 updates are necessary to make the cue visibility and policy-violated cue match the intended user experience.
|
|
4 participants
Fixes AB#580818
Requirement:
Update the following role centers:
-Accountant
-Accounts Payable Administrator
-Business Manager
-Business Manager Evaluation
We need the following cues:
-Opened Expense Reports
-Release Expense Reports
-Expense Reports Pending Approvals
-Approved Expense Reports
-Rejected Expense Reports
-Processed for Payment Exp. Rep.
-Completed Expense Reports
-Released Expenses
-Policy Violated Expenses