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

3078 otp expiration timer #3200

Merged
merged 2 commits into from
Apr 6, 2023
Merged

3078 otp expiration timer #3200

merged 2 commits into from
Apr 6, 2023

Conversation

cakekoa
Copy link
Contributor

@cakekoa cakekoa commented Apr 5, 2023

What does this PR do?

Fixes part of #3078 by displaying a countdown timer for the OTP expiration.

  • Displays a countdown timer for command expiration

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Testing Checklist

  • Added relevant unit tests?
  • Do all unit tests pass?
  • Do all end-to-end tests pass?
  • Any other testing performed?

    Tested by {Running the Monkey locally with relevant config/running Island/...}

  • If applicable, add screenshots or log transcripts of the feature working

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.05 ⚠️

Comparison is base (01dcc4f) 73.02% compared to head (3befef5) 72.97%.

❗ Current head 3befef5 differs from pull request most recent head 2f3b3ca. Consider uploading reports for the commit 2f3b3ca to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3200      +/-   ##
===========================================
- Coverage    73.02%   72.97%   -0.05%     
===========================================
  Files          469      471       +2     
  Lines        13566    13538      -28     
===========================================
- Hits          9907     9880      -27     
+ Misses        3659     3658       -1     

see 12 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mssalvatore mssalvatore changed the base branch from develop to 3078-refresh-on-copy April 5, 2023 23:19
Copy link
Collaborator

@mssalvatore mssalvatore left a comment

Choose a reason for hiding this comment

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

A video clip would help speed review along.

@ilija-lazoroski ilija-lazoroski force-pushed the 3078-otp-expiration-timer branch 2 times, most recently from 7dd721d to a94d30a Compare April 6, 2023 09:05
Copy link
Contributor

@ilija-lazoroski ilija-lazoroski left a comment

Choose a reason for hiding this comment

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

I would expect the command to be refreshed automatically if it is expired. Maybe that is not something we want but as a user experience, I wouldn't want to click Refresh after the timer expires.

Also lets add pie as expiration timer, not numbers..

Screen.Recording.2023-04-06.at.11.11.34.AM.mov

Comment on lines +10 to +11
if (minutes === 0 && seconds === 0) {
return 'Command expired';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we regenerate the command and restart the timer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: The useTimer method allows one to register an onExpire callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen.Recording.2023-04-06.at.7.38.29.AM.mov

@mssalvatore
Copy link
Collaborator

I would expect the command to be refreshed automatically if it is expired.

+1

@mssalvatore mssalvatore force-pushed the 3078-refresh-on-copy branch from fe3161d to 9cbefbf Compare April 6, 2023 13:52
@mssalvatore mssalvatore force-pushed the 3078-otp-expiration-timer branch from e89988b to 8e9c669 Compare April 6, 2023 13:59
Base automatically changed from 3078-refresh-on-copy to develop April 6, 2023 14:32
@mssalvatore mssalvatore force-pushed the 3078-otp-expiration-timer branch from 8e9c669 to 2f3b3ca Compare April 6, 2023 14:33
@mssalvatore mssalvatore marked this pull request as ready for review April 6, 2023 14:42
@mssalvatore mssalvatore merged commit c5cf872 into develop Apr 6, 2023
@mssalvatore mssalvatore deleted the 3078-otp-expiration-timer branch April 6, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants