Fix - Expense - Submit button appears for archived workspace chat if delayed submission is enabled #52183

FitseTLT · 2024-11-07T13:33:39Z

Details

Fixed Issues

$ #49169
PROPOSAL: #49169 (comment)

Tests

Create a workspace from User A and enable workflow and manual submission
Invite User B
Submit an expense report from User B's account
Remove User B from the workspace using User A's account
From User B side open the archive workspace chat
Open the expense report you submitted in (3)
Verify that composer is visible (not hidden) and you can comment, all context menu work correctly like reply in thread, react.
Verify that you can edit the expense fields like date, receipt, description, merchant, amount.
Verify that you cannot submit (submit button doesn't appear)
Verify that you cannot create expense on the archived expense report.

Verify that no errors appear in the JS console

Offline tests

Same as above

QA Steps

Same as above

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

2025-01-06.20-29-11.mp4

Android: mWeb Chrome

2025-01-06.20-36-52.mp4

iOS: Native

2025-01-06.20-48-38.mp4

iOS: mWeb Safari

2025-01-06.20-41-20.mp4

MacOS: Chrome / Safari

2025-01-06.20-50-26.mp4

MacOS: Desktop

2025-01-06.20-51-55.mp4

melvin-bot · 2024-11-07T13:33:44Z

@Ollyws 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]

src/components/MoneyReportHeader.tsx

src/libs/actions/IOU.ts

src/components/ReportActionItem/ReportPreview.tsx

srikarparsi · 2024-11-13T00:57:42Z

@FitseTLT what's the ETA for this PR? It's holding up a couple other PRs so just wanted to check. Also, we should try to be a little careful with this since it has a chance to cause regressions. By this, I mean that we should check all occurrences of isArchivedRoom and isArchivedRoomWithID to make sure that any that deal with expense reports are switched over to isArchivedExpenseReport. Otherwise, archived expense reports will be able to be Paid, Submitted or Approved which we don't want.

FitseTLT · 2024-11-13T21:53:18Z

@FitseTLT what's the ETA for this PR? It's holding up a couple other PRs so just wanted to check. Also, we should try to be a little careful with this since it has a chance to cause regressions. By this, I mean that we should check all occurrences of isArchivedRoom and isArchivedRoomWithID to make sure that any that deal with expense reports are switched over to isArchivedExpenseReport. Otherwise, archived expense reports will be able to be Paid, Submitted or Approved which we don't want.

I am working on it. Will provide update tomorrow 👍

FitseTLT · 2024-11-14T21:37:04Z

I have checked all instances of isArchivedRoom and isArchivedRoomWithID and I have listed down some case I want confirmation from U

I believe we don't want to hide edit, flag as offensive, delete action, join thread menu items for archive expense report.

App/src/components/MoneyReportHeader.tsx

Lines 253 to 258 in 58258ed

    
           if (!isArchivedReport) { 
        
               return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.bookingPendingDescription')}; 
        
           } 
        
           if (isArchivedReport && archiveReason === CONST.REPORT.ARCHIVE_REASON.BOOKING_END_DATE_HAS_PASSED) { 
        
               return {icon: getStatusIcon(Expensicons.Box), description: translate('iou.bookingArchivedDescription')}; 
        
           }

App/src/components/ReportActionItem/ReportPreview.tsx

Lines 369 to 372 in 58258ed

    
           if (!isArchivedReport) { 
        
               return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.bookingPending')}; 
        
           } 
        
           if (isArchivedReport && archiveReason === CONST.REPORT.ARCHIVE_REASON.BOOKING_END_DATE_HAS_PASSED) {

which I think you were discussing about above and you should help me what to do with them.

canAddTransaction, canDeleteTransaction, ??
getReasonAndReportActionThatRequiresAttention we return null if it is archived room or the parent report is.
Should it require attention?
getPolicyExpenseChatName , getReportName whether to show archived suffix
reasonForReportToBeInOptionList: Archived reports should always be shown when in default (most recent) mode
shouldShowFlagComment - we don't show flag comment page for archived pages should we
shouldReportShowSubscript - we don't show subtitle for archived reports, what about our for expense reports
canUserPerformWriteAction - I think we want to allow write action in order to allow commenting
shouldDisableThread - we want to enable reply in thread (let me know if you disagree
getOrderedReportIDs - we order the archived reports last in LHN
BaseReportActionContextMenu - whether to show hold and unhold menu in context menu
ReportActionItem - I don't think we want to disable emoji picker button in ReportActionItemMessageEdit
In report details page - we are currently showing cancel payment, unapproved, delete expense and oddly it works the BE correctly handles them. WDYT we should leave it ??

Now to confirm: You are aiming to set is_privateArchived for expense reports from BE. Correct me if I am wrong.

Last question: I know we want to allow commenting and so on on open archive expense reports but what about settled archived reports we want the same behaviour as the unsettled one? This might also decide our isArchivedExpenseReport.

srikarparsi · 2024-11-15T08:59:58Z

Thank you for all the questions @FitseTLT, I'll get to them tomorrow since I need to look at some of the backend code

srikarparsi · 2024-11-19T09:36:50Z

I believe we don't want to hide edit, flag as offensive, delete action, join thread menu items for archive expense report.

Yes, since they are still able to be commented on, we don't want to hide these menu items

What to do with ARCHIVE_REASON.BOOKING_END_DATE_HAS_PASSED

We do want to show this as archived. So the isArchivedReport should query for the last action and if it is this, then even if it's an expense report, then it should still be considered as archived.

canAddTransaction, canDeleteTransaction, ??

This should not be possible with isArchivedReport or isArchivedExpenseReport

getReasonAndReportActionThatRequiresAttention we return null if it is archived room or the parent report is. Should it require attention?

I think no right now because there is nothing actionable on these expense reports so they shouldn't have RBRs or GBRs. (The move button doesn't exist yet). But @trjExpensify could you help confirm?

getPolicyExpenseChatName , getReportName whether to show archived suffix

No, we should not show the archived suffix for expense reports

reasonForReportToBeInOptionList: Archived reports should always be shown when in default (most recent) mode

Yes, but for the expense reports, the reason should not be archived but rather the regular expense report reasons.

shouldShowFlagComment - we don't show flag comment page for archived pages should we

Yes, we should show it for expense reports because they are still able to be commented on

shouldReportShowSubscript - we don't show subtitle for archived reports, what about our for expense reports

Yes, we should show subscript, expense reports shouldn't be visibly archived in the frontend.

canUserPerformWriteAction - I think we want to allow write action in order to allow commenting

Yes.

shouldDisableThread - we want to enable reply in thread (let me know if you disagree

Yes, we want to enable reply in thread

getOrderedReportIDs - we order the archived reports last in LHN

Expense reports shouldn't be considered archived so we don't want to order these last

BaseReportActionContextMenu - whether to show hold and unhold menu in context menu

I don't believe we want to show hold and unhold either for archived expense reports. @trjExpensify do you think you can confirm this one as well.

ReportActionItem - I don't think we want to disable emoji picker button in ReportActionItemMessageEdit

Yes, we do not for archived expense reports.

In report details page - we are currently showing cancel payment, unapproved, delete expense and oddly it works the BE correctly handles them. WDYT we should leave it ??

I don't think we want to show any action buttons on archived expense reports. @trjExpensify can you please help confirm this one as well?

You are aiming to set is_privateArchived for expense reports from BE. Correct me if I am wrong.

Yes, but after we merge this PR so that users are able to continue commenting on expense reports. If we do that first before merging this PR, users won't be able to comment on expense reports until this PR gets deployed.

I know we want to allow commenting and so on on open archive expense reports but what about settled archived reports we want the same behaviour as the unsettled one?

Yes, I believe we want the same behavior for all expense reports. I had the same question here and @trjExpensify recommended we should retain the ability to chat on all archived expense reports.

Please let me know if you have any other questions.

FitseTLT · 2024-11-21T22:10:15Z

@srikarparsi Applied all changes. Only doubt here

App/src/components/ReportActionItem/ReportPreview.tsx

Line 364 in e9e1857

const isArchivedReport = ReportUtils.isArchivedRoomWithID(iouReportID);

App/src/components/MoneyReportHeader.tsx

Line 121 in e9e1857

    
           const isArchivedReport = ReportUtils.isArchivedRoomWithID(moneyRequestReport?.reportID);

We use the variable to determine the status bar prop. I know I asked you last time but it is not clear to me can you tell me directly what function to use, is it isArchivedNonExpenseReport or isArchivedAnyReport ?

trjExpensify · 2024-11-21T22:57:53Z

Sorry, I weirdly didn't catch this in my inbox until that latest comment.

I think no right now because there is nothing actionable on these expense reports so they shouldn't have RBRs or GBRs. (The move button doesn't exist yet). But @trjExpensify could you help confirm?

Agreed, no RBR/GBR. We should add the ability to change the workspace the expense report is on soon though!

BaseReportActionContextMenu - whether to show hold and unhold menu in context menu

I don't believe we want to show hold and unhold either for archived expense reports. @trjExpensify do you think you can confirm this one as well.

Hmm, I don't see a reason to restrict these.

In report details page - we are currently showing cancel payment, unapproved, delete expense and oddly it works the BE correctly handles them. WDYT we should leave it ??

I don't think we want to show any action buttons on archived expense reports. @trjExpensify can you please help confirm this one as well?

Same here, I think we should leave these as they are. OldDot doesn't restrict actions like reject, unapprove, delete etc when the workspace is deleted. Only Submit > Approve > Pay.

Yes, I believe we want the same behavior for all expense reports. I had the same question #49169 (comment) and @trjExpensify recommended we should retain the ability to chat on all archived expense reports.

Agreed.

FitseTLT · 2024-11-22T13:56:42Z

@srikarparsi I will need a confirmation on two of @trjExpensify responses and will make the changes as soon as you guys agree on it but the other responses are already aligned to the current code 👍
BaseReportActionContextMenu - whether to show hold and unhold menu in context menu

I don't believe we want to show hold and unhold either for archived expense reports. @trjExpensify do you think you can confirm this one as well.

Hmm, I don't see a reason to restrict these.

In report details page - we are currently showing cancel payment, unapproved, delete expense and oddly it works the BE correctly handles them. WDYT we should leave it ??

I don't think we want to show any action buttons on archived expense reports. @trjExpensify can you please help confirm this one as well?

Same here, I think we should leave these as they are. OldDot doesn't restrict actions like reject, unapprove, delete etc when the workspace is deleted. Only Submit > Approve > Pay.

srikarparsi · 2024-11-22T23:48:53Z

Thanks @trjExpensify and @FitseTLT. Let's move forward with what @trjExpensify said in this comment, I also agree with it.

srikarparsi · 2025-01-09T20:42:18Z

Creating an AdHoc build to test better

github-actions · 2025-01-09T20:42:22Z

🚧 @srikarparsi has triggered a test build. You can view the workflow run here.

github-actions · 2025-01-09T21:07:19Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/52183/index.html	❌ FAILED ❌
	The QR code can't be generated, because the iOS build failed
Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/52183/NewExpensify.dmg	https://52183.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

src/libs/actions/Task.ts

srikarparsi

Overall looks good to me other than the one comment I left. Also there's one conflict but other than that this looks good to merge.

FitseTLT · 2025-01-13T13:19:18Z

Conflict resolved.

srikarparsi · 2025-01-13T19:04:37Z

sorry, looks like there's more conflicts now. Once those are resolved I'll go ahead and merge this

srikarparsi · 2025-01-13T21:27:57Z

Eslint is now failing but I'm going to go ahead and merge this since it's a pretty large file diff and I want to keep moving forward with this since it also unblocks this Auth PR

srikarparsi

looks good overall, thanks for sticking with this

srikarparsi · 2025-01-13T21:30:07Z

Changed file ESLint was failing since a new import rule seems to have been imposed, explained in this comment.

OSBotify · 2025-01-13T21:38:26Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2025-01-14T13:59:29Z

🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.0.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

github-actions · 2025-01-14T14:15:30Z

🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.0.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

github-actions · 2025-01-14T14:40:52Z

🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.0.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

github-actions · 2025-01-14T15:24:33Z

🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.0.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

github-actions · 2025-01-14T15:48:44Z

🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.0.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	cancelled 🔪

izarutskaya · 2025-01-15T07:17:51Z

@FitseTLT Found issues - #55214, #55215 Repro in Prod
https://platform.applause.com/services/links/v1/external/8ac6b96ebb79b9e9f6ce0bbb76b633d672d50be910027147ce3b8e95b26347ed

github-actions · 2025-01-15T13:47:36Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.0.85-4 🚀

platform	result
🤖 android 🤖	true ❌
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	success ✅

trjExpensify · 2025-01-15T14:15:19Z

@FitseTLT @srikarparsi let's make sure to look into those!

github-actions · 2025-01-15T15:48:32Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.0.85-4 🚀

platform	result
🤖 android 🤖	true ❌
🖥 desktop 🖥	cancelled 🔪
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	success ✅

FitseTLT added 2 commits November 7, 2024 16:20

separated isArchivedExpenseReport

4f1e036

hide approve and submit button for archived expense reports

e479b03

FitseTLT requested a review from a team as a code owner November 7, 2024 13:33

melvin-bot bot requested a review from Ollyws November 7, 2024 13:33

melvin-bot bot removed the request for review from a team November 7, 2024 13:33

FitseTLT marked this pull request as draft November 7, 2024 13:33

FitseTLT mentioned this pull request Nov 7, 2024

[HOLD for payment 2025-01-22] [$500] Expense - Submit button appears for archived workspace chat if delayed submission is enabled #49169

Closed

6 tasks

srikarparsi reviewed Nov 10, 2024

View reviewed changes

src/components/MoneyReportHeader.tsx Show resolved Hide resolved

srikarparsi reviewed Nov 10, 2024

View reviewed changes

src/libs/actions/IOU.ts Outdated Show resolved Hide resolved

srikarparsi reviewed Nov 10, 2024

View reviewed changes

src/components/ReportActionItem/ReportPreview.tsx Show resolved Hide resolved

removed isArchived conditions

80079f1

FitseTLT requested a review from srikarparsi November 11, 2024 10:52

FitseTLT added 4 commits November 13, 2024 15:02

Merge branch 'main' into fix-expense-report-for-archived-workspaces

6baee2f

Merge branch 'main' into fix-expense-report-for-archived-workspaces

d1e30bb

removed unnecessary code

6e97db2

remove unnecessary condition

58258ed

Merge branch 'main' into fix-expense-report-for-archived-workspaces

e0da388

FitseTLT added 3 commits November 20, 2024 23:13

Merge branch 'main' into fix-expense-report-for-archived-workspaces

1f48cb9

updated isArchived conditions

a79e87b

changed function name

367f750

srikarparsi reviewed Jan 13, 2025

View reviewed changes

src/libs/actions/Task.ts Show resolved Hide resolved

srikarparsi requested changes Jan 13, 2025

View reviewed changes

Merge branch 'main' into fix-expense-report-for-archived-workspaces

4238c73

FitseTLT added 2 commits January 13, 2025 22:40

Merge branch 'main' into fix-expense-report-for-archived-workspaces

6e688e1

minor fix

7b22d11

srikarparsi approved these changes Jan 13, 2025

View reviewed changes

srikarparsi merged commit 9d6c88f into Expensify:main Jan 13, 2025
15 of 16 checks passed

melvin-bot bot added the Emergency label Jan 13, 2025

srikarparsi removed the Emergency label Jan 13, 2025

github-actions bot mentioned this pull request Jan 14, 2025

Deploy Checklist: New Expensify 2025-01-14 #55208

Closed

38 tasks

IuliiaHerets mentioned this pull request Jan 14, 2025

Unable to access submitted report in workspace chat when removed by admin #55214

Closed

8 tasks

vincdargento mentioned this pull request Jan 14, 2025

Report - Submit button for a report in an archived workspace can be accessed in reports #55215

Closed

8 tasks

srikarparsi mentioned this pull request Jan 14, 2025

[No QA] fix: Allow private_ prefix lint rule for variable naming #54937

Merged

50 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix - Expense - Submit button appears for archived workspace chat if delayed submission is enabled #52183

Fix - Expense - Submit button appears for archived workspace chat if delayed submission is enabled #52183

FitseTLT commented Nov 7, 2024 •

edited

Loading

melvin-bot bot commented Nov 7, 2024

srikarparsi commented Nov 13, 2024

FitseTLT commented Nov 13, 2024

FitseTLT commented Nov 14, 2024 •

edited

Loading

srikarparsi commented Nov 15, 2024

srikarparsi commented Nov 19, 2024 •

edited

Loading

FitseTLT commented Nov 21, 2024 •

edited

Loading

trjExpensify commented Nov 21, 2024

FitseTLT commented Nov 22, 2024 •

edited

Loading

srikarparsi commented Nov 22, 2024 •

edited

Loading

srikarparsi commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

srikarparsi left a comment

FitseTLT commented Jan 13, 2025

srikarparsi commented Jan 13, 2025

srikarparsi commented Jan 13, 2025

srikarparsi left a comment

srikarparsi commented Jan 13, 2025

OSBotify commented Jan 13, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

izarutskaya commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

trjExpensify commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

Fix - Expense - Submit button appears for archived workspace chat if delayed submission is enabled #52183

Fix - Expense - Submit button appears for archived workspace chat if delayed submission is enabled #52183

Conversation

FitseTLT commented Nov 7, 2024 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Nov 7, 2024

srikarparsi commented Nov 13, 2024

FitseTLT commented Nov 13, 2024

FitseTLT commented Nov 14, 2024 • edited Loading

srikarparsi commented Nov 15, 2024

srikarparsi commented Nov 19, 2024 • edited Loading

FitseTLT commented Nov 21, 2024 • edited Loading

trjExpensify commented Nov 21, 2024

FitseTLT commented Nov 22, 2024 • edited Loading

srikarparsi commented Nov 22, 2024 • edited Loading

srikarparsi commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

github-actions bot commented Jan 9, 2025

srikarparsi left a comment

Choose a reason for hiding this comment

FitseTLT commented Jan 13, 2025

srikarparsi commented Jan 13, 2025

srikarparsi commented Jan 13, 2025

srikarparsi left a comment

Choose a reason for hiding this comment

srikarparsi commented Jan 13, 2025

OSBotify commented Jan 13, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 14, 2025

izarutskaya commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

trjExpensify commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

FitseTLT commented Nov 7, 2024 •

edited

Loading

FitseTLT commented Nov 14, 2024 •

edited

Loading

srikarparsi commented Nov 19, 2024 •

edited

Loading

FitseTLT commented Nov 21, 2024 •

edited

Loading

FitseTLT commented Nov 22, 2024 •

edited

Loading

srikarparsi commented Nov 22, 2024 •

edited

Loading