Page MenuHomeSoftware Heritage

Add new bundle type to list only when connected as privileged user
ClosedPublic

Authored by ardumont on May 5 2021, 3:53 PM.

Details

Summary

This is only doing listing for now. It's in a dedicated commit so review is easier.

Some more work is required ui and server side to actually schedule those new visit type.
It will be done in a future commit [1]

[1] D5676 which will get reworked and based on this.

Related to T3213

Test Plan

tox

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 D5685 (id=20303)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit 9753e2c48bdf243c0cdb6d1efaec000f3e4490d6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when privileged
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Related to T3213

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

swh/web/auth/utils.py
74

privileged_user sounds like a better name for me.

swh/web/common/origin_save.py
140–141

I would name the parameter privileged_user, this is clearer.

355

same here

swh/web/common/swh_templatetags.py
128–139

This is only used for the "Take new snapshot" action so you can keep that function as it is.

swh/web/common/origin_save.py
153

the second call to list copy is not needed

ack on what you said, thanks.

I'll adapt as soon as i succeed in adding my last test about that privileged listing which is resisting me ;)

ardumont edited the summary of this revision. (Show Details)
  • Add missing tests (there remains one to uncomment and make green)
  • Adapt according to suggestions

Build has FAILED

Patch application report for D5685 (id=20307)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit 8a017d6ccadac03d440042feeb0858cabd9a8f35
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when privileged
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Related to T3213

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

ardumont retitled this revision from Add new bundle type to list only when privileged to Add new bundle type to list only when connected as privileged user.May 5 2021, 5:34 PM
ardumont edited the summary of this revision. (Show Details)

Build is green

Patch application report for D5685 (id=20307)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit 8a017d6ccadac03d440042feeb0858cabd9a8f35
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when privileged
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Related to T3213

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

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

so far... That won't see the user as an ambassador one...

>       assert set(actual_response) == set(PRIVILEGED_VISIT_TYPES)
E       AssertionError: assert {'git', 'hg', 'svn'} == {'bundle', 'git', 'hg', 'svn'}
E         Extra items in the right set:
E         'bundle'
E         Use -v to get the full diff
swh/web/tests/misc/test_origin_save.py
41–56

Because "origin-save-types-list" is not a DRF view so the bearer token auth will not work here.

You need to use the client fixture and login with client.login(code="", code_verifier="", redirect_uri="")

swh/web/tests/misc/test_origin_save.py
41–56

ah! ok
I bypassed it so far using a mock but i don't like that much.
So i'll give your suggestion a try ;)

Thanks.

  • Add the last test!
  • reword commit message
swh/web/tests/misc/test_origin_save.py
41–56

it worked ;)

swh/web/api/views/origin_save.py
88

you can save some lines by inlining privileged_user(request) in the function call

swh/web/common/origin_save.py
153

you can drop the call to list() here

swh/web/misc/origin_save.py
31–32

could be a one liner

Build is green

Patch application report for D5685 (id=20309)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit 0311b02a0d48ed990f9852bf9a3dc8db2ad6e73d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

ardumont marked 2 inline comments as done.

Adapt according to last suggestions

Build is green

Patch application report for D5685 (id=20323)

Rebasing onto 934cb65cdf...

Current branch diff-target is up to date.
Changes applied before test
commit bba3c8cb1ec47367cea6ec7fa91ad4725957f6f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

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

Build is green

Patch application report for D5685 (id=20334)

Rebasing onto 43636f47d5...

Current branch diff-target is up to date.
Changes applied before test
commit 1a2c53716cbeaedf7492702324fbcfbfd6c9ae18
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

Build is green

Patch application report for D5685 (id=20399)

Rebasing onto fcf53f1ec3...

First, rewinding head to replay your work on top of it...
Applying: Add new bundle type to list only when connected as privileged user
Changes applied before test
commit 6a1013d53a3f426c80e6fb2dee87857a9218ece6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

Build is green

Patch application report for D5685 (id=20402)

Rebasing onto fcf53f1ec3...

Current branch diff-target is up to date.
Changes applied before test
commit 11d7c4ef5fedac657953e1e610eb5a7ccab7c9c4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

Rebase with the correct commit range

Build is green

Patch application report for D5685 (id=20435)

Rebasing onto 1e97586914...

Current branch diff-target is up to date.
Changes applied before test
commit 28e5b7298047a6321a0ff4e486fb7d9d6f8081d4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:41:34 2021 +0200

    Allow privileged user to trigger save code now bundle visit type
    
    Related to T3213

commit 19452d1b88aace058902241b06db925ec1f65afe
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu May 6 14:05:15 2021 +0200

    tests: Add new ambassador user for frontend tests
    
    This refactor existing permission code to allow creation of user with some permissions.
    
    Related to T3213

commit 2a4f2e966a034e034a736e7e10b2b572ef32c89b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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

Build is green

Patch application report for D5685 (id=20436)

Rebasing onto 1e97586914...

Current branch diff-target is up to date.
Changes applied before test
commit 2a4f2e966a034e034a736e7e10b2b572ef32c89b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 5 15:25:33 2021 +0200

    Add new bundle type to list only when connected as privileged user
    
    This is only doing listing for now. It's in a dedicated commit so review is easier.
    
    Some more work is required ui and server side to actually schedule those new visit type.
    It will be done in a future commit.
    
    Related to T3213

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