Skip to content
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

[Hold for payment][$500] Chat - Video leads to not here page after reopening chat #36899

Closed
4 of 6 tasks
lanitochka17 opened this issue Feb 20, 2024 · 26 comments
Closed
4 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Feb 20, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.43-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Go to chat
  3. Upload a video
  4. Play the video and verify that it plays
  5. Go to LHN
  6. Reopen the chat in Step 2
  7. Play the video

Expected Result:

The video is played without issue

Actual Result:

The video leads to not here page

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6385618_1708429871937.Screen_Recording_20240220_195026_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~018ba9a1f89a9fa057
  • Upwork Job ID: 1759956026491719680
  • Last Price Increase: 2024-02-20
  • Automatic offers:
    • alitoshmatov | Reviewer | 0
    • bernhardoj | Contributor | 0
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor labels Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

Job added to Upwork: https://www.upwork.com/jobs/~018ba9a1f89a9fa057

@melvin-bot melvin-bot bot changed the title Chat - Video leads to not here page after reopening chat [$500] Chat - Video leads to not here page after reopening chat Feb 20, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

@melvin-bot melvin-bot bot added the Daily KSv2 label Feb 20, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 20, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Feb 20, 2024

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp
CC @quinthar

@bernhardoj
Copy link
Contributor

bernhardoj commented Feb 20, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

After reopening the chat page that has a video, pressing the video (on small screen) or expanding it will show not found page.

What is the root cause of that problem?

When the not found page shown, the reportID params in the URL is empty. We get the reportID from the active route.

const activeRoute = Navigation.getActiveRoute();
const {reportID} = parseReportRouteParams(activeRoute);
return (
<VideoPlayerPreview

But the reportID could be empty when the page is still transitioning. If the reportID is empty, we will see the not found page.

onShowModalPress={() => {
const route = ROUTES.REPORT_ATTACHMENTS.getRoute(reportID, sourceURL);
Navigation.navigate(route);
}}

What changes do you think we should make in order to solve the problem?

Get the reportID every time we press the video.

onShowModalPress={() => {
    const activeRoute = Navigation.getActiveRoute();
    const {reportID} = parseReportRouteParams(activeRoute);

    const route = ROUTES.REPORT_ATTACHMENTS.getRoute(reportID, sourceURL);
    Navigation.navigate(route);
}}

What alternative solutions did you explore? (Optional)

We can get the reportID from ShowContextMenuContext just like we did in ImageRenderer.

<ShowContextMenuContext.Consumer>
{({anchor, report, action, checkIfContextMenuActive}) => (
<PressableWithoutFocus
style={[styles.noOutline]}
onPress={() => {
const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report?.reportID ?? '', source);
Navigation.navigate(route);
}}

@stitesExpensify
Copy link
Contributor

This is coming from #30829

@francoisl do you think we should accept the above proposal? Or would you rather wait until next week after the deploy freeze to handle this?

@francoisl
Copy link
Contributor

Yep, we should still try to address deploy blockers during the freeze. If the fix works, let's go with it.

@stitesExpensify
Copy link
Contributor

Great. @alitoshmatov what do you think of the above proposal?

Copy link

melvin-bot bot commented Feb 21, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@alitoshmatov
Copy link
Contributor

@bernhardoj 's proposal does solve this issue and provide correct RCA

We can go with this proposal

C+ reviewed 🎀 👀 🎀

Copy link

melvin-bot bot commented Feb 21, 2024

Current assignee @stitesExpensify is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

📣 @alitoshmatov 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Feb 21, 2024

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Feb 22, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @alitoshmatov

@francoisl francoisl removed the DeployBlockerCash This issue or pull request should block deployment label Feb 23, 2024
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Mar 18, 2024
Copy link

melvin-bot bot commented Mar 18, 2024

This issue has not been updated in over 15 days. @stitesExpensify, @bernhardoj, @alitoshmatov eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@alitoshmatov
Copy link
Contributor

Looks like Melvin failed here and we missed this issue. The PR went to into production 3 weeks ago. Can we proceed with next step

@stitesExpensify

@stitesExpensify
Copy link
Contributor

Does the issue still exist on production? If so then yes let's proceed!

@alitoshmatov
Copy link
Contributor

@stitesExpensify This issue does not exist in production. The payment is not handled yet

@stitesExpensify stitesExpensify removed the External Added to denote the issue can be worked on by a contributor label Mar 22, 2024
@stitesExpensify
Copy link
Contributor

Oh, my apologies I misunderstood your question. I will get a bug zero member assigned!

@stitesExpensify stitesExpensify added the External Added to denote the issue can be worked on by a contributor label Mar 22, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 22, 2024
@stitesExpensify stitesExpensify added Bug Something is broken. Auto assigns a BugZero manager. and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Mar 22, 2024
Copy link

melvin-bot bot commented Mar 22, 2024

Current assignee @alitoshmatov is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Mar 22, 2024
Copy link

melvin-bot bot commented Mar 22, 2024

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@stitesExpensify
Copy link
Contributor

Hi @sonialiap Melvin didn't trigger here, but the issue is already completed. Could you please process the payment for @bernhardoj and @alitoshmatov ?

@sonialiap
Copy link
Contributor

Thanks for the note about payment, processing now

Payment summary:

@sonialiap sonialiap changed the title [$500] Chat - Video leads to not here page after reopening chat [Hold for payment][$500] Chat - Video leads to not here page after reopening chat Mar 26, 2024
@sonialiap sonialiap removed the Reviewing Has a PR in review label Mar 26, 2024
@alitoshmatov
Copy link
Contributor

@sonialiap Accepted the offer

@sonialiap
Copy link
Contributor

Both payments made ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants