-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Allow admin to create report on behalf of employee #76000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
luacmartins
merged 30 commits into
Expensify:main
from
s77rt:report-transaction-new-report
Jan 23, 2026
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
9624ed2
allow admin to create report on behald of employee
s77rt 2f31cbd
Merge branch 'main' into report-transaction-new-report
s77rt a2a4db8
lint
s77rt a8c2633
pass accountID to CreateAppReport
s77rt 9123cb9
pass owner personal details on report creation
s77rt e390506
get owner from selected report
s77rt 84983e5
allow editing unreported managed card transaction
s77rt 23e156f
make changeTransactionsReport accept transactions
s77rt b790fdb
add transaction to SelectedTransactionInfo
s77rt 1dcfb45
pass transactions to changeTransactionsReport
s77rt 2b2077f
update CreateAppReportParams to use reportCreateEmail
s77rt 1692887
lint
s77rt 7dbb1be
merge main
s77rt 7efe983
handle unreported expenses
s77rt 8a12936
add comment
s77rt a5225d7
rename param reportCreatorEmail to ownerEmail
s77rt f49d612
rename variable
s77rt 607b09d
lint
s77rt e1727f0
lint
s77rt 0eb413b
lint
s77rt 6ad170f
merge main
s77rt 30e9927
remove transaction from SelectedTransactionInfo
s77rt ffc3100
Merge branch 'main' into report-transaction-new-report
s77rt f4aaced
more robust condition
s77rt 260ea01
merge main
s77rt 8f0b167
merge main
s77rt 60ebd76
Merge branch 'main' into report-transaction-new-report
s77rt 24b9b01
get transactions from SelectedTransactions object
s77rt 1d3e080
lint
s77rt 6659725
merge main
s77rt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This early return grants edit permission for any unreported managed card transaction without verifying the current user is a policy admin or manager. That bypasses all of the existing role checks below, so any user who can see an unreported managed card transaction would be allowed to edit it, which contradicts the comment and widens access. Consider gating this on policy admin/manager status (or reuse existing permission helpers) before returning true.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the domain admin would have higher access than a policy admin, so the current code change seems correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@s77rt Should we restrict report creation to only admins or owners? Currently this still works fine since transactions are only visible to the report's admin and owner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is fine. As you pointed out, unreported transactions won't show up unless you can act on them. Also I don't have a solution in mind to check for domain admin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% confident this is correct, so I'm a bit worried it might introduce fragile code. @luacmartins could you give your thoughts on this matter?