Page MenuHomeSoftware Heritage

origin_save: Enable to filter user submitted save requests in Web UI
ClosedPublic

Authored by anlambert on May 4 2021, 6:27 PM.

Details

Summary

If a user has submitted save code now requests while being authenticated,
he will be able to filter those requests through a new dedicated checkbox
in the "Browse save requests" tab of the Save code now Web UI.

Related to T3272

Depends on D5679 (I manually hack the Jenkins pipeline until it is landed)

Diff Detail

Repository
rDWAPPS Web applications
Branch
user-save-code-now-requests-list
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21313
Build 33099: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 33098: arc lint + arc unit

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 4 2021, 6:32 PM
Harbormaster failed remote builds in B21255: Diff 20268!

Build has FAILED

Patch application report for D5673 (id=20268)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit f548dd7006c4b0a696ac513b0f5affb2b1197cf0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Build has FAILED

Patch application report for D5673 (id=20268)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit f548dd7006c4b0a696ac513b0f5affb2b1197cf0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Build has FAILED

Patch application report for D5673 (id=20268)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit f548dd7006c4b0a696ac513b0f5affb2b1197cf0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Put test user creation in a separate file

Build has FAILED

Patch application report for D5673 (id=20268)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit f548dd7006c4b0a696ac513b0f5affb2b1197cf0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Build has FAILED

Patch application report for D5673 (id=20286)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit 68f265aa47901aa51bef045f969c480cafba577e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Harbormaster returned this revision to the author for changes because remote builds failed.May 5 2021, 12:28 PM
Harbormaster failed remote builds in B21272: Diff 20286!

Build has FAILED

Patch application report for D5673 (id=20286)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit 68f265aa47901aa51bef045f969c480cafba577e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

Build is green

Patch application report for D5673 (id=20286)

Rebasing onto f8c750b65a...

Current branch diff-target is up to date.
Changes applied before test
commit 68f265aa47901aa51bef045f969c480cafba577e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

assets/src/bundles/save/index.js
79–81

that weird syntax set the datatables UI components plus an extra one while not breaking the bootstrap layout.
(see comments in https://datatables.net/examples/advanced_init/dom_toolbar.html)

ardumont added inline comments.
assets/src/bundles/save/index.js
79–81

yes, thanks, I recall datatable are highly configurable and some with weird syntax ;)

Maybe inline your comment in the code directly so we can refer to it next time we encouter and scratch our heads ;)

swh/web/api/views/origin_save.py
90 ↗(On Diff #20286)

oops ;)

lgtm, a couple of question/suggestion and one spurious print to drop inline ;)

swh/web/tests/misc/test_origin_save.py
28

I did not immediately get what you added at the end (connecting the
user and filtering only on their input).
Might be describing a bit the test would be nice?

This revision is now accepted and ready to land.May 5 2021, 2:54 PM

(I did not finish reviewing, but I'm marking this as "request changes" now so you don't merge it immediately)

No API endpoint for this feature?


Possibly a dumb question, but why do we have classes like this:

<div class="custom-control custom-checkbox swhid-option">
  <input class="custom-control-input">
  <label class="custom-control-label" ...>

Can't the CSS refer to custom-control input and custom-control label instead of custom-control-input and custom-control-label?

assets/src/bundles/save/index.js
40

might be a bit clearer like this

79–81

I was about to ask about this.

I still don't understand what is happening here after reading that page. Could you explain it with a comment in the code?

cypress/integration/origin-save.spec.js
429–469

could you add some comments inside this test to explain the steps?

swh/web/api/views/origin_save.py
90 ↗(On Diff #20286)

forgot to remove this

swh/web/templates/misc/origin-save.html
99–100
vlorentz requested changes to this revision.May 5 2021, 2:57 PM
This revision now requires changes to proceed.May 5 2021, 2:57 PM

No API endpoint for this feature?

Ah right we could add a /origin/save/requests endpoint that lists submitted requests in a paginated way with query parameters to filter the results in a similar way than the Web UI.

This could be a nice easy hack task.

I'm done now.

Last comment: when someone creates a Save request and it remains in status "not yet scheduled", and a logged in user clicks "Save again", it does not appear when this user selects "show only your requests".
I did not try with other statuses, though.

Is it on purpose?

I'm done now.

Last comment: when someone creates a Save request and it remains in status "not yet scheduled", and a logged in user clicks "Save again", it does not appear when this user selects "show only your requests".
I did not try with other statuses, though.

Is it on purpose?

Yes, if someone tries to create a request for an origin but there is already one in pending state, the request will not be created again.
Proper solution would be to store a list of user ids in the save request model using a django ArrayField.

anlambert added inline comments.
assets/src/bundles/save/index.js
79–81

sure

cypress/integration/origin-save.spec.js
429–469

ack, @ardumont also found some were missing.

swh/web/api/views/origin_save.py
90 ↗(On Diff #20286)

whoopsie

swh/web/templates/misc/origin-save.html
99–100

ack

swh/web/tests/misc/test_origin_save.py
28

In fact I was just lazy to write a new test function but I should have.

Proper solution would be to store a list of user ids in the save request model using a django ArrayField.

Unfortunately, this is PostgreSQL specific. I guess I can use a TextField then and concatenates the user ids separated by commas.

Can't the CSS refer to custom-control input and custom-control label instead of custom-control-input and custom-control-label?

I just followed bootstrap doc here.

Update: Adapt according to reviews

Build is green

Patch application report for D5673 (id=20308)

Rebasing onto 934cb65cdf...

First, rewinding head to replay your work on top of it...
Applying: origin_save: Enable to filter user submitted save requests in Web UI
Changes applied before test
commit 9295176739ede407cc0c753c08a16b8fc2cdde86
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

swh/web/common/migrations/0011_saveoriginrequest_user_ids.py
1–22

Note that migration number 10 did not get deployed to production yet, it could be removed and that migration will become number 10.

Build is green

Patch application report for D5673 (id=20313)

Rebasing onto 934cb65cdf...

First, rewinding head to replay your work on top of it...
Applying: origin_save: Enable to filter user submitted save requests in Web UI
Changes applied before test
commit c820e3e5b3f280a5b5afbe19665aee7216196462
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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

This revision is now accepted and ready to land.May 6 2021, 10:52 AM

Rebase and improve if condition

Build is green

Patch application report for D5673 (id=20331)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit 43636f47d5626fb56d99a90b64e10bf39de9f6d1
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue May 4 14:50:53 2021 +0200

    origin_save: Enable to filter user submitted save requests in Web UI
    
    If a user has submitted save code now requests while being authenticated,
    he will be able to filter those requests through a new dedicated checkbox
    in the "Browse save requests" tab of the Save code now Web UI.
    
    Related to T3272

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