Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f741c4b78
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const actions = [...(parentReportAction ? [parentReportAction] : []), ...getSortedReportActions(Object.values(reportActions ?? {}))]; | ||
| for (const action of actions) { | ||
| if (!isReportActionVisible(action, reportID, canUserPerformAction, visibleReportActionsData) || isMoneyRequestAction(action)) { | ||
| for (const [key, action] of actions.entries()) { | ||
| if (!shouldReportActionBeVisible(action, key, canUserPerformAction) || isMoneyRequestAction(action)) { |
There was a problem hiding this comment.
Pass reportActionID to visibility check
shouldReportActionBeVisible relies on its key to detect deprecated actions via isReportActionDeprecated, which compares String(reportAction.sequenceNumber) to the key. Here the key comes from actions.entries() so it is just the array index, not the reportActionID, meaning any action whose sequenceNumber happens to equal its position will be treated as deprecated and skipped. In reports with sequential sequenceNumbers this can drop otherwise-visible actions and their attachments. Use action.reportActionID (or the original reportActions object key) instead of the index.
Useful? React with 👍 / 👎.
|
@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Straight revert |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.7-3 🚀
|
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.7-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.8-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.8-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.11-16 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.3.12-1 🚀
|
Reverts #78634 to fix deploy blockers:
Straight revert