Skip to content

fix: clicking submit button makes flicking on a workspace with delay … #53541

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.

Sign up for GitHub

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
rafecolton merged 3 commits into from
Dec 10, 2024
Merged

fix: clicking submit button makes flicking on a workspace with delay … #53541

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89dc9f5
fix: clicking submit button makes flicking on a workspace with delay …
jacobkim9881 Dec 4, 2024
f41129b
Merge branch 'main' of https://github.com/Expensify/App into m53226
jacobkim9881 Dec 5, 2024
4a52096
Merge branch 'main' of https://github.com/Expensify/App into m53226
jacobkim9881 Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libs/Middleware/SaveResponseInOnyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const requestsToIgnoreLastUpdateID: string[] = [
SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP,
WRITE_COMMANDS.CLOSE_ACCOUNT,
WRITE_COMMANDS.DELETE_MONEY_REQUEST,
WRITE_COMMANDS.SUBMIT_REPORT,

@arosiclair arosiclair Dec 19, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafecolton @rojiphil @jacobkim9881 hey I noticed this change while investigating another issue. I don't have full context on this bug, but this is almost certainly the wrong way to fix it. We're essentially disabling reliable Onyx updates for SubmitReport requests here and ignoring any Onyx updates that may have been missed. I think we should revert this change and find another way to address the root problem.

@rafecolton rafecolton Dec 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 thanks Andrew. I agree that does not sound great, and I did not realize that's what this change would do. Let's revert. I commented on the issue.

@arosiclair arosiclair Dec 19, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries and thanks for raising the PR 🙇. Changes to this file aren't always obvious so maybe we should add a MelvinBot comment with a warning when a change is detected

@rafecolton rafecolton Dec 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that would be a good improvement

@jacobkim9881 jacobkim9881 Dec 20, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arosiclair Thanks for reviewing. It makes me find an issue.

@rojiphil rojiphil Dec 20, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing. It makes me find an issue.

@jacobkim9881 Not sure if I understand. Can you please share the issue you found due to the change here?

This comment was marked as off-topic.

Sign in to view

@jacobkim9881 jacobkim9881 Dec 25, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing. It makes me find an issue.

@rojiphil It is revealed as unrelated from my side.

SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES,
];

Expand Down