Open vincdargento opened 3 weeks ago
Triggered auto assignment to @JmillsExpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
π¨ Edited by proposal-police: This proposal was edited at 2025-01-24 13:48:38 UTC.
Copilot - No countdown and code requested message on magic code page when changing access level
We have not implemented the timer feature in BaseValidateCodeForm.tsx as we are doing here
shouldShowTimer
)
https://github.com/Expensify/App/blob/95cc6482377130f923bf9970d81a0dcf5f1e2774/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx#L224shouldShowTimer
is false setTimeRemaining(CONST.REQUEST_CODE_DELAY);
N/A as it is a UI change
We can create hook that returns remainingTime and a function that resets the timer following the same logic as the existing timer implementation.
const {remainingTime, resetTimer} = useRequestTimeout();
And use remainingTime
to decide if should display timer, and call resetTimer
when requesting a new code to reset the time.
Tests
remainingTime
should be 30 sec when calling the hookremainingTime
should be 0 after 30 second remaingTime
should be 30 sec when resetTimer is calledπ¨ Edited by proposal-police: This proposal was edited at 2025-01-24 17:25:44 UTC.
Thereβs no countdown and "Code Requested" message displayed on the Magic Code page when changing the access level.
In UpdateDelegateMagicCodePage Component
We are using an independent ValidateCodeForm (src/pages/settings/Security/AddDelegate/UpdateDelegateRole/ValidateCodeForm/index.tsx), but the code to display the countdown timer has not been implemented
Note: In all other instances, we consistently use the ValidateCodeActionModal.
Therefore, I suggest that in the UpdateDelegateMagicCodePage component, we also use ValidateCodeActionModal, as weβve done in other places like DelegateMagicCodeModal, etc.
I believe this section is unnecessary
No
β οΈ @cretadn22 Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).
@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!
Job added to Upwork: https://www.upwork.com/jobs/~021884532777502086335
Opened up to the community.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jayeshmangwani (External
)
Thank you for the proposals above.
I agree with @cretadn22 's Proposal to use the ValidateCodeActionModal
for consistency with other pages that use a timer for the magic code.
Let's go with their proposal.
π π π C+ reviewed
Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
π£ @cretadn22 π 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 π
@JmillsExpensify, @jayeshmangwani, @aldo-expensify, @cretadn22 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
We're waiting on @cretadn22 to raise the PR.
I'm currently working on this
@jayeshmangwani Take a look at this video. When an incorrect magic code is entered, the optimistic value updates initially, but once the backend returns a failure, the old value is reverted, causing a flickering effect. Since updating the delegate role requires a correct code that can only be verified by the backend, we shouldn't use optimistic data for this action. Instead, we could consider displaying a loading spinner while waiting for the API response. This might be out of scope for now, but it would be a valuable improvement.
https://github.com/user-attachments/assets/ef685443-7b94-4702-8bf1-252cf65c7cc6
Thanks for the input, @cretadn22 , I checked a few cases where we use the magic input, and it flickers in the current production version. So, I think we can include this change in this PR to improve the visual effect.
@cretadn22 @jayeshmangwani agree with adding that in scope if easy. @cretadn22 when can we expect PR for review?
Almost done, @jayeshmangwani Please start reviewing. On the other way, I will update this additional point later today
I get this error because of many API calls. It block me from completing this PR
@jayeshmangwani All code changes are complete. Please proceed with the review. I will run another test and upload videos once the API is working again.
@cretadn22 Maybe you can try with a new account, or you could wait an hour before calling this API again.
Please proceed with the review
Yes sure, I'll review PR
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.97-1 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2025-02-19. :confetti_ball:
For reference, here are some details about the assignees on this issue:
@jayeshmangwani @JmillsExpensify @jayeshmangwani The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
[x] [Contributor] 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: N/A
[x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: I don't think this is a regression; rather, it's a design improvement to make it consistent with other Magic code pages.
[x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
countdown
and the message Code requested. Please check your device
should be displayed.Do we agree π or π
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: 9.0.89-2 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+100106kh@applause.expensifail.com Issue reported by: Applause Internal Team Device used: Mac 15.0 / Chrome App Component: User Settings
Action Performed:
Expected Result:
On magic code page when changing access level for existing copilot, it should display a countdown and "Code requested. Please check your device." message.
Actual Result:
On magic code page when changing access level for existing copilot, it does not display a countdown and "Code requested. Please check your device." message, which is not consistent with the magic code page in Step 6.
Workaround:
Unknown
Platforms:
Screenshots/Videos
https://github.com/user-attachments/assets/a29c7f9e-9f59-4d32-82df-f3a4526fb96c
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @JmillsExpensify