Fix approve button not shown because of invisible broken card connection violation by rayane-d · Pull Request #74738 · Expensify/App

rayane-d · 2025-11-10T20:17:56Z

Explanation of Change

Problem

When a submitter dismisses an RTER (broken card connection) violation on an expense report, the "Approve" button on the report preview doesn't appear for the approver, even though the approver should not see this violation at all.

Root Cause

The issue has two parts:

Visibility Logic: TransactionUtils.shouldShowViolation() correctly hides RTER violations from approvers (non-submitters) on non-instant-submit policies:

App/src/libs/TransactionUtils/index.ts

Lines 1225 to 1228 in d82a640

    
           if (violationName === CONST.VIOLATIONS.RTER) { 
        
               return (isSubmitter || isInstantSubmitEnabled(policy)) && (shouldShowRterForSettledReport || !isSettled(iouReport)); 
        
           }

This means approvers should never see RTER violations on non instant-submit policies.

Approve Action Availability Logic: ReportPreviewActionUtils.canApprove() was checking ReportUtils.hasAnyViolations() which:
- Only filters violations dismissed by the current user (the approver)
- Since the submitter dismissed it (not the approver), it is considered when determining if the Approve button should be displayed to the approver
- Even though the violation is invisible to the approver, it still blocks the Approve button

App/src/libs/ReportPreviewActionUtils.ts

Line 66 in d82a640

    
           const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations);

So the issue is that we were checking if violations exist, but not checking if those violations are visible to the current user.

Solution

This PR ensures that "Approve" action availability logic in ReportPreviewActionUtils.canApprove() only considers violations visible to the current user by properly integrating the dismissal and visibility logic.

1. Enhanced `isViolationDismissed()` to support all Auth dismissal rules

The function now implements the same three conditions as the Auth backend:

https://github.com/Expensify/Auth/blob/6005bc71c5ee81f4e89f9a70535d853b19eb57a5/auth/lib/Violations.cpp#L430-L451

The violation is considered dismissed if:

Current user dismissed it themselves
Admin viewing OPEN report AND report owner dismissed it
RTER violation on instant submit policy - dismissed by anyone

2. Fixed "Approve" action availability logic in `ReportPreviewActionUtils.canApprove()` to respect violation visibility

Updated to use hasVisibleViolationsForUser() which checks both:

Is violation dismissed? (via isViolationDismissed)
Should user see it? (via shouldShowViolation)

For the reported bug:

Submitter dismisses RTER violation on non instant-submit policy
Report is submitted
Approver views the report
shouldShowViolation() returns false because:
- Approver is not submitter
- Policy is not instant submit
hasVisibleViolationsForUser() returns false (no visible violations)
Approve button appears as expected

Before:

After:

3. Fixed empty "Next Step" message for dismissed violations

Problem:
When all violations were dismissed, the header tried to display a status message but useTransactionViolations filtered them out, resulting in an empty status bar.

Fix:
Added check to ensure violations exist before showing status:

if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) {
    return {
        icon: getStatusIcon(Expensicons.Hourglass),
        description: <BrokenConnectionDescription ... />
    };
}

Before:

After:

4. Fixed "Mark as cash" primary action button on the report header instead of "Approve" button

Problem:

isMarkAsCashAction was using allHavePendingRTERViolation and shouldShowBrokenConnectionViolationForMultipleTransactions to display the "Mark as cash" button when it returns true. allHavePendingRTERViolation and shouldShowBrokenConnectionViolationForMultipleTransactions was considering all non-dismissed violations, it:

Only filters violations dismissed by the current user (the approver)
Since the submitter dismissed it (not the approver), it is considered when determining if the "isMarkAsCashAction" button should be displayed to the approver
Even though the violation is invisible to the approver, it still shows the "isMarkAsCashAction" button

App/src/libs/ReportPrimaryActionUtils.ts

Line 279 in d82a640

    
           const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations);

App/src/libs/TransactionUtils/index.ts

Lines 1236 to 1250 in d82a640

    
           /** 
        
            * Check if there is pending rter violation in all transactionViolations with given transactionIDs. 
        
            */ 
        
           function allHavePendingRTERViolation(transactions: OnyxEntry<Transaction[] | SearchTransaction[]>, transactionViolations: OnyxCollection<TransactionViolations> | undefined): boolean { 
        
               if (!transactions) { 
        
                   return false; 
        
               } 
        
               const transactionsWithRTERViolations = transactions.map((transaction) => { 
        
                   const filteredTransactionViolations = getTransactionViolations(transaction, transactionViolations); 
        
                   return hasPendingRTERViolation(filteredTransactionViolations); 
        
               }); 
        
               return transactionsWithRTERViolations.length > 0 && transactionsWithRTERViolations.every((value) => value === true); 
        
           }

App/src/libs/ReportPrimaryActionUtils.ts

Line 289 in d82a640

    
           const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations);

App/src/libs/TransactionUtils/index.ts

Lines 1163 to 1179 in d82a640

    
           /** 
        
            * Check if user should see broken connection violation warning based on selected transactions. 
        
            */ 
        
           function shouldShowBrokenConnectionViolationForMultipleTransactions( 
        
               transactionIDs: string[], 
        
               // eslint-disable-next-line @typescript-eslint/no-deprecated 
        
               report: OnyxEntry<Report> | SearchReport, 
        
               policy: OnyxEntry<Policy>, 
        
               transactionViolations: OnyxCollection<TransactionViolation[]>, 
        
           ): boolean { 
        
               const violations = transactionIDs.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []); 
        
               const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation)); 
        
               return shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations, report, policy); 
        
           }

Fix:

Updated allHavePendingRTERViolation() and shouldShowBrokenConnectionViolationForMultipleTransactions to check for both:

Is violation dismissed? (via isViolationDismissed)
Should user see it? (via shouldShowViolation)

Before:

After:

Fixed Issues

$ #73098
PROPOSAL:

Tests

Download and import the onyx state from this comment: https://expensify.slack.com/archives/C05LX9D6E07/p1760755153206519?thread_ts=1760583420.629759&cid=C05LX9D6E07
Run this in the browser console to add missing comment data to the transaction and to correct the violation name:

Onyx.merge('transactions_8142574694853799211', {
  "comment": {
    "attendees": [
      {
        "avatarUrl": "https://d1wpcgnaa73g0y.cloudfront.net/b99046a36bab1955c1669e891375a845d544a4fb_128.jpeg",
        "displayName": "Jasper Huang",
        "email": "jasper@expensify.com"
      }
    ],
    "comment": "***",
    "dismissedViolations": {
      "rter": {
        "jasper@expensify.com": 1760643377364493
      }
    }
  }
});
  
  
Onyx.set(`transactionViolations_8142574694853799211`, [{
      "data": {
          "rterType": "brokenCardConnection",
          "tooltip": "Personal Cards: Fix your card from Account Settings. Corporate Cards: ask your Expensify admin to fix your company's card connection."
      },
      "name": "rter",
      "showInReview": true,
      "type": "warning"
}]);

Open the policy expense chat (report #96832091)
Verify that the report preview action shows an "Approve" button
Open the expense report (#1814181775632892)
Verify that the primary action button is "Approve"
Verify that there is no empty report next step message (there should be no next step for this one expense report)

Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Same as tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

…n violation

…ction violation

…provide an email to isViolationDismissed

…ep is dismissed

…missed

codecov · 2025-11-11T14:06:56Z

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.

Files with missing lines	Coverage Δ
...thSections/Search/TransactionGroupListExpanded.tsx	`57.47% <100.00%> (ø)`
src/hooks/useTransactionViolations.ts	`100.00% <100.00%> (ø)`
src/libs/ReportPrimaryActionUtils.ts	`89.16% <100.00%> (-0.06%)`	⬇️
src/libs/ReportSecondaryActionUtils.ts	`90.93% <100.00%> (-0.03%)`	⬇️
src/libs/ReportUtils.ts	`74.21% <100.00%> (-0.03%)`	⬇️
src/libs/SearchUIUtils.ts	`69.22% <100.00%> (+0.03%)`	⬆️
src/libs/TransactionPreviewUtils.ts	`73.48% <100.00%> (+0.59%)`	⬆️
src/libs/Violations/ViolationsUtils.ts	`66.98% <100.00%> (ø)`
...ionListWithSections/Search/TransactionListItem.tsx	`2.12% <0.00%> (ø)`
src/hooks/useTransactionsAndViolationsForReport.ts	`68.75% <66.66%> (+4.46%)`	⬆️
... and 5 more
... and 89 files with indirect coverage changes

mountiny · 2025-11-11T15:49:57Z

src/libs/TransactionUtils/index.ts

+        return !!transaction?.comment?.dismissedViolations?.[violation.name];
+    }
+
    return !!transaction?.comment?.dismissedViolations?.[violation.name]?.[currentUserEmail ?? deprecatedCurrentUserEmail];


the deprecatedCurrentUserEmail is not needed now?

lets add / update tests for this method

the deprecatedCurrentUserEmail is not needed now?

It was added in #72940. Based on this comment, @DylanDylann might be working on a PR to remove it. @DylanDylann, can you please confirm?

Yeah but if currentUserEmail is falsey we will never get there so we could remove it from this line now, right?

I agree, I've removed it 👍

currentUserEmail could still be undefined, so let's keep deprecatedCurrentUserEmail as a fallback for now. I'm still working on completely removing deprecatedCurrentUserEmail.

melvin-bot · 2025-11-11T16:26:29Z

@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

mountiny

Interesting, feels like this might have influenced many flows so I wonder if we are not missing anything about how violations are supposed to work, @pecanoro @cead22 do you think you could give this PR/ RCA a review too please?

cead22 · 2025-11-11T17:42:38Z

This change ensures that violations dismissed by a user no longer incorrectly appear active for other users.

This isn't right for all cases. For instance for the duplicatedExpense violation if the submitter dismisses it, the approver should still see it, and vice versa. Let's make sure to test these two cases in this PR

This isViolationDismissed() is used to determine if we can display the approve button (this is the root cause of the reported bug) here:

This also doesn't sound right, as you should be able to submit reports with violations, unless you're on a domain group with Strictly enforce expense workspace rules enabled (only available in Classic)

As for the broken connection violation, search for dismissedByEmail in Auth's auth/lib/Violations.cpp so you can see the different conditions

…se for duplicatedExpense violation, if the submitter dismisses it, the approver should still see it, and vice versa

rayane-d · 2025-11-11T20:46:54Z

This change ensures that violations dismissed by a user no longer incorrectly appear active for other users.

This isn't right for all cases. For instance for the duplicatedExpense violation if the submitter dismisses it, the approver should still see it, and vice versa. Let's make sure to test these two cases in this PR

I checked all isViolationDismissed usages, and I believe the only concern we need to address is with the duplicatedExpense violation (great catch, @cead22!), specifically here:

App/src/libs/TransactionUtils/index.ts

Line 1320 in 5dc4f0c

    
           const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation);

I've pushed a commit to handle this case: 4871b1f

In all the following isViolationDismissed usages, we pass the current user's email as a parameter:

App/src/components/SelectionListWithSections/Search/TransactionListItem.tsx

Line 107 in 5dc4f0c

    
           !isViolationDismissed(transactionItem, violation, currentUserDetails.email ?? '') &&

App/src/hooks/useTransactionViolations.ts

Line 29 in 5dc4f0c

    
           !isViolationDismissed(transaction, violation, currentUserDetails.email ?? '') &&

App/src/libs/SearchUIUtils.ts

Line 920 in 5dc4f0c

    
           return transactionViolations.filter((violation) => !isViolationDismissed(transaction, violation, currentUserEmail));

App/src/libs/TransactionUtils/index.ts

Line 1081 in 5dc4f0c

(violation) => !isViolationDismissed(transaction, violation, currentUserEmail),

In the following usages, we don't pass the email parameter, which is intentional because we want to check if the violation has been dismissed by any user:

App/src/libs/TransactionUtils/index.ts

Line 1369 in 5dc4f0c

!isViolationDismissed(transaction, violation),

App/src/libs/TransactionUtils/index.ts

Line 1397 in 5dc4f0c

!isViolationDismissed(transaction, violation),

App/src/libs/TransactionUtils/index.ts

Line 1418 in 5dc4f0c

!isViolationDismissed(transaction, violation),

This isViolationDismissed() is used to determine if we can display the approve button (this is the root cause of the reported bug) here:

This also doesn't sound right, as you should be able to submit reports with violations, unless you're on a domain group with Strictly enforce expense workspace rules enabled (only available in Classic)

To clarify, isViolationDismissed() is used within the hasAnyViolations logic. Then hasAnyViolations is used in canApprove to return false only if there are violations and shouldConsiderViolations is true:

App/src/libs/ReportPreviewActionUtils.ts

Line 85 in 5dc4f0c

    
           return isExpense && isProcessing && !!isApprovalEnabled && (!hasAnyViolations || !shouldConsiderViolations) && reportTransactions.length > 0 && isCurrentUserManager;

As for the broken connection violation, search for dismissedByEmail in Auth's auth/lib/Violations.cpp so you can see the different conditions

The logic in this PR aligns with this comment in Auth:

https://github.com/Expensify/Auth/blob/d6c4017a8bf00f477db56ad3cadf8863a9e054b3/auth/lib/Violations.cpp#L439-L440

// RTER violations on instant submit reports only need to be dismissed by one person to be considered dismissed

You can also see a log here showing that the approver did receive the dismissed violation data:

https://logs.expensify.com/goto/f57262f0-bf3d-11f0-aa21-6b7684ca60b0

pecanoro · 2025-11-20T19:26:49Z

@Krishna2323 Can you re-review the PR and test it to make sure it works well?

Krishna2323 · 2025-11-20T19:28:25Z

I will start reviewing this in an hour.

Krishna2323 · 2025-11-20T19:48:22Z

Reviewing...

Krishna2323 · 2025-11-20T20:39:27Z

@rayane-d one report preview shows a “Review” button when there’s a violation in the preview, but on the expense details page I don’t see any violation.

Another report preview shows a “View” button even though the expense has a violation.

Are both of these expected?

Monosnap.screencast.2025-11-21.01-51-41.mp4

rayane-d · 2025-11-20T20:47:27Z

@Krishna2323 - This is because the Onyx exported state contains masked data - specifically, the violation.name and transaction.comment fields are masked. That's why I included steps in the PR test instructions to correct the data for that transaction and violation. To test other transactions, you will need to execute snippets in the console to correct masked data for them.

Krishna2323 · 2025-11-20T20:49:49Z

Hmm 🤔, I'm pretty sure that I executed snippets in the console. Will try again, maybe I did something wrong.

…enCardConnection-violation

Krishna2323 · 2025-11-21T11:15:41Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android_native.mp4

Android: mWeb Chrome

android_chrome.mp4

iOS: HybridApp

ios_native.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

web_chrome.mp4

MacOS: Desktop

desktop_app.mp4

Krishna2323

Everything looks good and works well. I wasn’t able to test on mWeb Safari, but I don’t think that’s a blocker.

@rayane-d could you please fix the ESLint check? Thanks!

…enCardConnection-violation

rayane-d · 2025-11-21T14:37:51Z

@rayane-d could you please fix the ESLint check? Thanks!

Fixed

pecanoro · 2025-11-22T00:14:02Z

@cead22 All yours!

cead22 · 2025-11-24T19:41:53Z

src/libs/TransactionUtils/index.ts

 import type {Attendee, Participant, SplitExpense} from '@src/types/onyx/IOU';
 import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon';
 import type {OnyxData} from '@src/types/onyx/Request';
+// eslint-disable-next-line @typescript-eslint/no-deprecated


Suggested change

// eslint-disable-next-line @typescript-eslint/no-deprecated

// eslint-disable-next-line @typescript-eslint/no-deprecated

cead22 · 2025-11-24T19:42:53Z

src/libs/TransactionUtils/index.ts

+            // Further filter to only violations visible to the current user
+            shouldShowViolation(report, policy, violation.name, currentUserEmail),
+        );
+        // Check if there is pending rter violation in the filtered violations


Suggested change

// Check if there is pending rter violation in the filtered violations

// Check if there is pending rter violation in the filtered violations

cead22 · 2025-11-24T19:43:58Z

src/libs/TransactionUtils/index.ts

+    currentUserEmail: string,
+    iouReport: OnyxEntry<Report>,
+    policy: OnyxEntry<Policy>,
+    // eslint-disable-next-line @typescript-eslint/no-deprecated


Suggested change

// eslint-disable-next-line @typescript-eslint/no-deprecated

// eslint-disable-next-line @typescript-eslint/no-deprecated

DylanDylann · 2025-11-25T16:37:35Z

src/libs/TransactionPreviewUtils.ts

    const hasFieldErrors = hasMissingSmartscanFields(transaction);
    const isPaidGroupPolicy = isPaidGroupPolicyUtil(iouReport);
-    const hasViolationsOfTypeNotice = hasNoticeTypeViolation(transaction, violations, true) && isPaidGroupPolicy;
+    const currentUserEmail = getCurrentUserEmail();


Don't use getCurrentUserEmail function anymore please, please pass data from UI. I will fix in here

DylanDylann · 2025-11-25T16:41:14Z

src/libs/TransactionUtils/index.ts

+        return false;
+    }
+
+    const currentUserAccountID = deprecatedCurrentUserAccountID;


@rayane-d why are we still using deprecatedCurrentUserAccountID here even though it’s marked as deprecated?

cc @Krishna2323

@rayane-d @Krishna2323 Could you please create a PR to remove the usage of deprecatedCurrentUserAccountID?

Sorry about that, @DylanDylann — I got confused between deprecatedCurrentUserAccountID and deprecatedCurrentUserEmail. #74738 (comment)

@rayane-d are you available to raise the PR, or should I?

When a variable is marked as deprecated like deprecatedCurrentUserAccountID and deprecatedCurrentUserEmail, please don't introduce new usage of it anymore BUT if it already been used before and you only update that logic, It's fine to keep it.

Also, please avoid using functions like getCurrentUserEmail that rely on Onyx.connect. Onyx.connect is deprecated and needs to be fully removed from our app. This means functions like getCurrentUserEmail will also be removed, so please don’t use them in new code.

@rayane-d @Krishna2323 Could you please create a PR to remove the usage of deprecatedCurrentUserAccountID?

Working on a PR 👍

A LOT of places to update: #76171 WIP

IuliiaHerets · 2025-11-26T02:08:06Z

@rayane-d can you please share QA steps?

OSBotify · 2025-11-26T03:53:19Z

🚀 Deployed to staging by https://github.com/cead22 in version: 9.2.64-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Krishna2323 · 2025-11-26T08:51:17Z

@IuliiaHerets the QA steps should be the same as the tests section. Sorry for missing that.

cc: @rayane-d

IuliiaHerets · 2025-11-26T14:15:17Z

@Krishna2323 We don’t have access to the link from Step 1. Pasting the code into the console doesn’t generate a report as described in Step 3. It seems the steps are still incorrect

bandicam.2025-11-26.22-14-04-470.mp4

rayane-d · 2025-11-26T19:37:41Z

@Krishna2323 We don’t have access to the link from Step 1. Pasting the code into the console doesn’t generate a report as described in Step 3. It seems the steps are still incorrect

bandicam.2025-11-26.22-14-04-470.mp4

@IuliiaHerets I've DMed you the file in Slack 👍

OSBotify · 2025-11-27T01:21:47Z

🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.64-5 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

rayane-d added 2 commits November 10, 2025 21:16

fix approve button not shown because of invisible brokenCardConnectio…

981b9b0

…n violation

fix empty report next step shown because of invisible brokenCardConne…

c198f7b

…ction violation

melvin-bot bot assigned rayane-d Nov 10, 2025

rayane-d mentioned this pull request Nov 10, 2025

[Due for payment 2025-12-04] [$250] Investigate incorrect button showing when report is ready to be approved #73098

Closed

8 tasks

rayane-d added 4 commits November 11, 2025 13:29

Just check if the violation has been dismissed by anyone If we don't …

5bb1c92

…provide an email to isViolationDismissed

Fix empty report next step message when the broken connection next st…

e3d73cb

…ep is dismissed

revert a change

b03bdb1

fix empty report next step when the BrokenConnection violation is dis…

b5bf0ca

…missed

mountiny reviewed Nov 11, 2025

View reviewed changes

rayane-d added 2 commits November 11, 2025 17:14

run prettier

2f7c119

add automated tests for isViolationDismissed

7cc00cc

rayane-d requested a review from mountiny November 11, 2025 16:19

rayane-d marked this pull request as ready for review November 11, 2025 16:26

rayane-d requested review from a team as code owners November 11, 2025 16:26

melvin-bot bot requested review from Krishna2323 and removed request for a team November 11, 2025 16:26

melvin-bot bot requested review from joekaufmanexpensify and removed request for a team November 11, 2025 16:26

mountiny previously approved these changes Nov 11, 2025

View reviewed changes

mountiny requested review from cead22 and pecanoro November 11, 2025 16:45

pass current user email to isViolationDismissed in isDuplicate. becau…

4871b1f

…se for duplicatedExpense violation, if the submitter dismisses it, the approver should still see it, and vice versa

rayane-d dismissed mountiny’s stale review via 4871b1f November 11, 2025 20:46

rayane-d added 2 commits November 11, 2025 22:03

fix failing test

6924d6b

fix type errors

9cc99ea

rayane-d requested a review from Krishna2323 November 19, 2025 23:46

Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…

82cfbd2

…enCardConnection-violation

Krishna2323 approved these changes Nov 21, 2025

View reviewed changes

melvin-bot bot requested a review from mountiny November 21, 2025 11:18

rayane-d added 2 commits November 21, 2025 15:28

fix lint-changed errors

2719db5

Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…

394a446

…enCardConnection-violation

pecanoro approved these changes Nov 22, 2025

View reviewed changes

cead22 approved these changes Nov 24, 2025

View reviewed changes

cead22 merged commit 40f4ca2 into Expensify:main Nov 24, 2025
31 checks passed

rayane-d deleted the fix-approve-button-not-shown-because-of-brokenCardConnection-violation branch November 24, 2025 20:08

DylanDylann reviewed Nov 25, 2025

View reviewed changes

OSBotify mentioned this pull request Nov 26, 2025

Deploy Checklist: New Expensify 2025-11-26 #76071

Closed

70 tasks

Krishna2323 mentioned this pull request Nov 26, 2025

Fix: Expenses moved to Self DM, display a Join button on top of report #76043

Merged

53 tasks

rayane-d mentioned this pull request Nov 26, 2025

Remove deprecatedCurrentUserAccountID in src/libs/TransactionUtils/index.ts #76171

Merged

53 tasks

Krishna2323 mentioned this pull request Dec 1, 2025

Payment issue for E/App PR#76171 #76401

Closed

	if (violationName === CONST.VIOLATIONS.RTER) {
	return (isSubmitter \|\| isInstantSubmitEnabled(policy)) && (shouldShowRterForSettledReport \|\| !isSettled(iouReport));
	}

	/**
	* Check if there is pending rter violation in all transactionViolations with given transactionIDs.
	*/
	function allHavePendingRTERViolation(transactions: OnyxEntry<Transaction[] \| SearchTransaction[]>, transactionViolations: OnyxCollection<TransactionViolations> \| undefined): boolean {
	if (!transactions) {
	return false;
	}

	const transactionsWithRTERViolations = transactions.map((transaction) => {
	const filteredTransactionViolations = getTransactionViolations(transaction, transactionViolations);
	return hasPendingRTERViolation(filteredTransactionViolations);
	});
	return transactionsWithRTERViolations.length > 0 && transactionsWithRTERViolations.every((value) => value === true);
	}

	/**
	* Check if user should see broken connection violation warning based on selected transactions.
	*/
	function shouldShowBrokenConnectionViolationForMultipleTransactions(
	transactionIDs: string[],
	// eslint-disable-next-line @typescript-eslint/no-deprecated
	report: OnyxEntry<Report> \| SearchReport,
	policy: OnyxEntry<Policy>,
	transactionViolations: OnyxCollection<TransactionViolation[]>,
	): boolean {
	const violations = transactionIDs.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []);

	const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation));

	return shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations, report, policy);
	}

	// Check if there is pending rter violation in the filtered violations

	// Check if there is pending rter violation in the filtered violations

Conversation

rayane-d commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Problem

Root Cause

Solution

1. Enhanced isViolationDismissed() to support all Auth dismissal rules

2. Fixed "Approve" action availability logic in ReportPreviewActionUtils.canApprove() to respect violation visibility

3. Fixed empty "Next Step" message for dismissed violations

4. Fixed "Mark as cash" primary action button on the report header instead of "Approve" button

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov bot commented Nov 11, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melvin-bot bot commented Nov 11, 2025

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

cead22 commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rayane-d commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pecanoro commented Nov 20, 2025

Uh oh!

Krishna2323 commented Nov 20, 2025

Uh oh!

Krishna2323 commented Nov 20, 2025

Uh oh!

Krishna2323 commented Nov 20, 2025

Uh oh!

rayane-d commented Nov 20, 2025

Uh oh!

Krishna2323 commented Nov 20, 2025

Uh oh!

Krishna2323 commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

Krishna2323 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rayane-d commented Nov 21, 2025

Uh oh!

pecanoro commented Nov 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rayane-d commented Nov 10, 2025 •

edited

Loading

1. Enhanced `isViolationDismissed()` to support all Auth dismissal rules

2. Fixed "Approve" action availability logic in `ReportPreviewActionUtils.canApprove()` to respect violation visibility

codecov bot commented Nov 11, 2025 •

edited by github-actions bot

Loading

cead22 commented Nov 11, 2025 •

edited

Loading

rayane-d commented Nov 11, 2025 •

edited

Loading

Krishna2323 commented Nov 21, 2025 •

edited

Loading

Krishna2323 left a comment •

edited

Loading

Krishna2323 commented Nov 26, 2025 •

edited

Loading

IuliiaHerets commented Nov 26, 2025 •

edited

Loading