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.

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

Jump to bottom

simplify file upload statuses #196

Merged
merged 3 commits into from
Feb 25, 2021
Merged

simplify file upload statuses #196

merged 3 commits into from
Feb 25, 2021

Conversation

dcmcand
Copy link

@dcmcand dcmcand commented Feb 25, 2021

Description of change
Simplified file statuses presented to user. The mapping is now

UPLOADING -> Uploading
UPLOADED -> Uploaded
UPLOAD_FAILED, QUEUING_FAILED -> Upload Failed
SCANNING_QUEUED, SCANNING -> Scanning
APPROVED -> Approved
REJECTED -> Rejected.

Once HHS#317 is complete, then QUEUING_FAILED will be moved to Scanning since it will have retry for failed queuing.

How to test

  1. Pull down changes.
  2. Start the app
  3. While the clamav instance is still starting add a grantee, click save draft, and upload a file
  4. File status should display 'Uploaded'
  5. Refresh the page, File status should display 'Scanning'
  6. Wait for ClamAV to finish starting and for a success message from the worker.
  7. Refresh the page and status should now say 'Approved'
  8. Stop the redis instance (docker stop head-start-ttadp_redis_1)
  9. Upload a new file
  10. Status should be uploaded. It will stay uploaded until the request to que times out (approx 2 mins)
  11. Go make a cup of coffee or tea
  12. Come back and refresh the page. If status is still 'Updated ' then the request hasn't timed out yet
  13. Refresh the page, status should now be 'Upload Failed'
    Note: if you start redis again within the timeout period then queuing will succeed and the job will get processed, so we currently have some resiliency there for dropouts.
    Issue(s)

Checklist

  • Meets issue criteria
  • Code tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • Documentation updated

jasalisbury
jasalisbury approved these changes Feb 25, 2021
View reviewed changes
Copy link

@jasalisbury jasalisbury left a comment

Choose a reason for hiding this comment

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

👍

frontend/src/components/FileUploader.js
@@ -78,6 +78,30 @@ Dropzone.propTypes = {
id: PropTypes.string.isRequired,
};

export const getStatus = (status) => {
switch (status) {
case 'UPLOADING':
Copy link

Choose a reason for hiding this comment

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

Should we put these statuses in a constant somewhere?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I was wondering about that. certainly could, I did on the backend.

Copy link

Choose a reason for hiding this comment

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

I'm fine with leaving these here if it's the only place they're referenced. This function basically becomes the constant.

@dcmcand dcmcand requested a review from rahearn February 25, 2021 16:06
rahearn
rahearn approved these changes Feb 25, 2021
View reviewed changes
frontend/src/components/FileUploader.js
@@ -78,6 +78,30 @@ Dropzone.propTypes = {
id: PropTypes.string.isRequired,
};

export const getStatus = (status) => {
switch (status) {
case 'UPLOADING':
Copy link

Choose a reason for hiding this comment

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

I'm fine with leaving these here if it's the only place they're referenced. This function basically becomes the constant.

@dcmcand dcmcand merged commit 68a69d1 into main Feb 25, 2021
@dcmcand dcmcand deleted the cm-310-update-file-statuses branch February 25, 2021 16:39
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
c165a41 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
71834d6 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
2835cf6 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
ef2168b 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
fe10580 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
55e2e57 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
26887e4 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
dcmcand added a commit that referenced this pull request Feb 25, 2021
@dcmcand
Squashed commit of the following:
16be599 
commit 68a69d1
Merge: f9408ad 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 11:39:32 2021 -0500

    Merge pull request #196 from adhocteam/cm-310-update-file-statuses

    simplify file upload statuses

commit 7f4d157
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:43:33 2021 -0500

    fmt

commit b7724ce
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:39:19 2021 -0500

    add tests for getStatus()

commit cc12546
Author: Chuck McAndrew <[email protected]>
Date:   Thu Feb 25 10:04:59 2021 -0500

    simplify file upload statuses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasalisbury jasalisbury jasalisbury approved these changes

@rahearn rahearn rahearn approved these changes

@dcloud dcloud Awaiting requested review from dcloud

@gopar gopar Awaiting requested review from gopar

@kryswisnaskas kryswisnaskas Awaiting requested review from kryswisnaskas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dcmcand @rahearn @jasalisbury