You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2024-04-09] [HOLD for payment 2024-03-13] [$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before #36873
Version Number: 1.4.43-0 Reproducible in staging?: Y Reproducible in production?: N Logs:https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team Slack conversation:
Action Performed:
Prerequisites: The user should be logged in and a video exist in the chat
Go to prerequisite chat
Play&Pause the video
Tap on the expand button on the video preview
Play&Pause the video
Exit from the attachment view via clicking X button top right
Try to Play&Pause the video in preview mode
Expected Result:
Play&Pause buttons function correctly
Actual Result:
Play&Pause buttons don't function in preview mode and console error occurs
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
changed the title [-]Video - Play&Pause buttons don't function in preview mode if expanded the video before[/-][+][$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/+]on Feb 20, 2024
👋 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:
Identify the pull request that introduced this issue and revert it.
changed the title [-][HOLD for payment 2024-03-13] [$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/-][+][HOLD for payment 2024-04-09] [HOLD for payment 2024-03-13] [$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/+]on Apr 2, 2024
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
[@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
[@ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@ishpaul777] Determine if we should create a regression test for this bug.
[@ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@miljakljajic] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
[@ishpaul777] The PR that introduced the bug has been identified. Link to the PR: #30829
[@ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: #30829 (comment)
[@ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: not required
[@ishpaul777] Determine if we should create a regression test for this bug. - yes
[@ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
Regression test Proposal
Prerequisites: The user should be logged in and a video exist in the chat
Go to prerequisite chat
Play&Pause the video, verify it works as expected
Tap on the expand button on the video preview
Play&Pause the video, verify it works as expected
Exit from the attachment view via clicking X button top right
Try to Play&Pause the video in preview mode, verify it works as expected
Open Video in full screen mode, Try to Play&Pause the video in preview mode, verify it works as expected
i dont think the regression was from the initial PR directly, the RC was much complex than initially expected, also the only a small part was reverted from the first PR so i feel there shouldn't a penalty in this case, @lakchote can you please help with evaluation here as assigned engineer
i dont think the regression was from the initial PR directly, the RC was much complex than initially expected, also the only a small part was reverted from the first PR so i feel there shouldn't a penalty in this case, @lakchote can you please help with evaluation here as assigned engineer
Even though the RC was complex to fix, this comes directly from the initial PR related to the video player and files concerned (BaseVideoPlayer for instance). This case should have been identified and fixed from the beginning, even from just a functional standpoint.
That's why, since it's a regression, I think 50% payment seems fair to me.
Activity
[-]Video - Play&Pause buttons don't function in preview mode if expanded the video before[/-][+][$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/+]melvin-bot commentedon Feb 20, 2024
Job added to Upwork: https://www.upwork.com/jobs/~013093a3e364e94fed
melvin-bot commentedon Feb 20, 2024
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ishpaul777 (
External)melvin-bot commentedon Feb 20, 2024
Triggered auto assignment to @miljakljajic (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.github-actions commentedon Feb 20, 2024
👋 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:
58 remaining items
[-][HOLD for payment 2024-03-13] [$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/-][+][HOLD for payment 2024-04-09] [HOLD for payment 2024-03-13] [$500] Video - Play&Pause buttons don't function in preview mode if expanded the video before[/+]melvin-bot commentedon Apr 2, 2024
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Apr 2, 2024
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-04-09. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Apr 2, 2024
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
ishpaul777 commentedon Apr 7, 2024
[@ishpaul777] The PR that introduced the bug has been identified. Link to the PR: #30829
[@ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: #30829 (comment)
[@ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: not required
[@ishpaul777] Determine if we should create a regression test for this bug. - yes
[@ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
Regression test Proposal
Prerequisites: The user should be logged in and a video exist in the chat
Do we agree 👍 or 👎 ?
miljakljajic commentedon Apr 9, 2024
Is payment for this issue docked by 50% for the regression?
ishpaul777 commentedon Apr 9, 2024
i dont think the regression was from the initial PR directly, the RC was much complex than initially expected, also the only a small part was reverted from the first PR so i feel there shouldn't a penalty in this case, @lakchote can you please help with evaluation here as assigned engineer
melvin-bot commentedon Apr 9, 2024
Payment Summary
Upwork Job
BugZero Checklist (@miljakljajic)
lakchote commentedon Apr 10, 2024
Even though the RC was complex to fix, this comes directly from the initial PR related to the video player and files concerned (
BaseVideoPlayerfor instance). This case should have been identified and fixed from the beginning, even from just a functional standpoint.That's why, since it's a regression, I think 50% payment seems fair to me.
ishpaul777 commentedon Apr 10, 2024
OkY no problem 🙂 Thanks for the evaluation @lakchote
miljakljajic commentedon Apr 15, 2024
Paid 250 via upwork to @ishpaul777 - thank you all