Page MenuHomeSoftware Heritage

vault/assets: Fix cooking task creation when there is a pending one
ClosedPublic

Authored by anlambert on Nov 23 2022, 4:50 PM.

Details

Summary

When requesting a vault cooking creation but there is already a pending one
for the same SWHID, ensure task creation modal is displayed as previously an
error was erroneoulsy reported in the Web UI.

Under the hood, no new cooking task will be created but the pending one will
be displayed in the vault UI. Nevertheless the email possibly submitted by the
user in the modal will be added to the list of emails to notify by the vault
backend.

Related to T4698

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D8878 (id=32008)

Could not rebase; Attempt merge onto b04d5848c6...

Updating b04d5848..a566268c
Fast-forward
 cypress/e2e/origin-save.cy.js              |  2 +-
 cypress/e2e/vault.cy.js                    | 19 +++++++++++++++++++
 swh/web/vault/assets/vault-create-tasks.js | 11 ++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)
Changes applied before test
commit a566268c24f55d8334b49ea3801f4a281b9af3ca
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Report pending cooking task when submitting the same one
    
    Previously an error was erroneoulsy reported in the Web UI.
    
    Related to T4698

commit cd820470fa59192f318afc081d00e80c25fe2cc6
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 15:08:34 2022 +0100

    cypress/origin-save: Fix flaky test

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/5/ for more details.

thanks!

Is it visible on the Downloads page even if someone else requested the cooking?

thanks!

Is it visible on the Downloads page even if someone else requested the cooking?

Ah right, need to check.

thanks!

Is it visible on the Downloads page even if someone else requested the cooking?

Ah right, need to check.

So the answer is no. It also makes me think that if a cooking task is pending, we still need to display the modal for creating a task as the user can ask for email notification.
I checked vault source code and when there is a pending cooking task, it will not be recreated but the provided email will be added to a list of emails to notify so fix is in fact
simpler than what I thought.

Rework fix to ensure cooking task will be displayed in vault UI.

anlambert retitled this revision from vault/assets: Report pending cooking task when submitting the same one to vault/assets: Fix cooking task creation when there is a pending one.Nov 24 2022, 2:30 PM
anlambert edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Nov 24 2022, 2:32 PM
vlorentz added inline comments.
swh/web/vault/assets/vault-create-tasks.js
52

wait, what happens if response.ok is true here? No feedback to the user?

This revision now requires changes to proceed.Nov 24 2022, 2:33 PM

Build is green

Patch application report for D8878 (id=32020)

Rebasing onto cd820470fa...

Current branch diff-target is up to date.
Changes applied before test
commit f3b1786b639b1c9fa2b4b86eb18f87a3aa36b1be
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/9/ for more details.

Handle all possible cooking statuses.

swh/web/vault/assets/vault-create-tasks.js
52

I updated the diff to handle all possible vault cooking statuses and the danger alert should only be displayed when HTTP response ends up with error so we should be good now.

Build is green

Patch application report for D8878 (id=32021)

Rebasing onto cd820470fa...

Current branch diff-target is up to date.
Changes applied before test
commit c07f9ce462e8dba2df300ed66131bd32245436a3
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/10/ for more details.

swh/web/vault/assets/vault-create-tasks.js
52

Could you make it display "something unexpected happened" and send a Sentry report when none of the conditions is satisfied, then?

I don't like the idea of not giving any feedback when the backend sends an unexpected response

swh/web/vault/assets/vault-create-tasks.js
52

I removed the !response.ok condition. We do not have any caught exception here so I do not think it worth it to report to sentry as Web API errors are already reported.

Build was aborted

Patch application report for D8878 (id=32022)

Rebasing onto cd820470fa...

Current branch diff-target is up to date.
Changes applied before test
commit e04412c408ece4e009f2360768ca04b31e05f537
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/11/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/11/console

Build has FAILED

Patch application report for D8878 (id=32023)

Rebasing onto cd820470fa...

Current branch diff-target is up to date.
Changes applied before test
commit c32edb667d1b118ca973f5f3965a97b202ec80ea
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/12/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/12/console

This revision is now accepted and ready to land.Nov 24 2022, 3:57 PM

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto cd820470fa...

Current branch diff-target is up to date.
Changes applied before test
commit c32edb667d1b118ca973f5f3965a97b202ec80ea
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/13/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/13/console

This revision was landed with ongoing or failed builds.Nov 24 2022, 4:48 PM
This revision was automatically updated to reflect the committed changes.

Build has FAILED

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-14-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/14/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/14/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-15-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/15/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/15/console

Build has FAILED

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-16-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/16/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/16/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-17-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/17/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/17/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-18-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/18/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/18/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-19-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/19/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/19/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-20-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/20/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/20/console

Build was aborted

Patch application report for D8878 (id=32023)

Rebasing onto c32edb667d...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-21-D8878.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/21/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/21/console