Page MenuHomeSoftware Heritage
Differential D2660

Adding 'Add Origin info' checkbox which was missing
ClosedPublic
Actions

Authored by Iamshankhadeep on Feb 12 2020, 2:49 PM.

Details

Reviewers
vlorentz
anlambert
Group Reviewers
Reviewers
Commits
rDWAPPS20a6073cd4d5: browse/directory: Add missing 'Add origin info' checkbox in permalinks tab
Summary

But if we browse the directory through its identifier and the origin info is provided as query parameter (https://archive.softwareheritage.org/browse/directory/ec4ae097465d9ea51589537ea94b2ea50e8d134d/?origin=https://hal.archives-ouvertes.fr/hal-02309043), the "Add origin info" checkbox is missing.
Related to T2141

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

Iamshankhadeep created this revision.Feb 12 2020, 2:49 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 12 2020, 2:49 PM
vlorentz added a subscriber: vlorentz.Feb 12 2020, 2:52 PM

Could you add a test for this, to prevent regressions in the future?

swh-public-ci added a comment.Feb 12 2020, 2:58 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/928/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/928/console

Harbormaster failed remote builds in B10573: Diff 9512!Feb 12 2020, 2:58 PM
vlorentz added a comment.Feb 12 2020, 3:06 PM

Don't worry about the build failure, it's temporary and not related to your changes

swh-public-ci added a comment.Feb 12 2020, 3:08 PM

Build has FAILED

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

vlorentz added a comment.Feb 12 2020, 3:30 PM

I fixed the build issue in master, and retriggered the builds of this diff. The errors should disappear.

swh-public-ci added a comment.Feb 12 2020, 3:39 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/930/ for more details.

swh-public-ci added a comment.Feb 12 2020, 3:55 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/539/ for more details.

Harbormaster completed remote builds in B10573: Diff 9512.Feb 12 2020, 3:55 PM
Iamshankhadeep updated this revision to Diff 9559.Feb 15 2020, 2:34 PM
  • Adding 'Add Origin info' checkbox which was missing
  • Adding tests to check permalink-box content
swh-public-ci added a comment.Feb 15 2020, 2:43 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/955/ for more details.

swh-public-ci added a comment.Feb 15 2020, 2:50 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/564/ for more details.

Harbormaster completed remote builds in B10620: Diff 9559.Feb 15 2020, 2:50 PM
vlorentz requested changes to this revision.Feb 17 2020, 1:15 PM

Thanks!

Just one comment:

swh/web/tests/browse/views/test_directory.py
73

I would rather you test swh-id-option-origin-directory, which checks the presence of the field instead of the form. For now it's the same thing, but we might add other fields in the future.

This revision now requires changes to proceed.Feb 17 2020, 1:15 PM
anlambert requested changes to this revision.Feb 17 2020, 1:43 PM
anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/web/tests/browse/views/test_directory.py
62–68

The test looks good but let's try to avoid hardcoding directory sha1 and origin url.
You can use the following to do so:

@given(directory())
def test_permalink_box_context(client, tests_data, directory):
    origin_url = random.choice(tests_data['origins'])['url']
    url = reverse('browse-directory',
                  url_args={'sha1_git': directory},
                  query_params={'origin': origin_url})

    resp = client.get(url)

    assert resp.status_code == 200
    assert_contains(resp, '<form id="swh-id-options">')
73

I agree with @vlorentz

Iamshankhadeep updated this revision to Diff 9569.Feb 17 2020, 2:38 PM
  • removed hardcoded data and updated test_directory.py as asked
swh-public-ci added a comment.Feb 17 2020, 2:47 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/963/ for more details.

swh-public-ci added a comment.Feb 17 2020, 2:54 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/572/ for more details.

Harbormaster completed remote builds in B10630: Diff 9569.Feb 17 2020, 2:54 PM
anlambert requested changes to this revision.Feb 17 2020, 3:18 PM

Looks good to me ! Before I can accept the diff, please rebase to master, squash the commits and use a better commit message, something like:

browse/directory: Add missing 'Add origin info' checkbox in permalinks tab

Closes T2141
This revision now requires changes to proceed.Feb 17 2020, 3:18 PM
Iamshankhadeep updated this revision to Diff 9617.Feb 18 2020, 4:16 PM

rebase to origin/master and squash commits

Iamshankhadeep awarded a token.Feb 18 2020, 4:18 PM
anlambert accepted this revision.Feb 18 2020, 4:21 PM
vlorentz accepted this revision.Feb 18 2020, 4:23 PM
This revision is now accepted and ready to land.Feb 18 2020, 4:23 PM
swh-public-ci added a comment.Feb 18 2020, 4:24 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/974/ for more details.

swh-public-ci added a comment.Feb 18 2020, 4:30 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/583/ for more details.

Harbormaster completed remote builds in B10678: Diff 9617.Feb 18 2020, 4:30 PM
Closed by commit rDWAPPS20a6073cd4d5: browse/directory: Add missing 'Add origin info' checkbox in permalinks tab (authored by Iamshankhadeep). · Explain WhyFeb 18 2020, 4:35 PM
This revision was automatically updated to reflect the committed changes.
Iamshankhadeep added a commit: rDWAPPS20a6073cd4d5: browse/directory: Add missing 'Add origin info' checkbox in permalinks tab.

Revision Contents
Changeset List

PathSize
swh/
web/
browse/
views/
10 lines
tests/
browse/
views/
15 lines

Diff 9619

View Options

swh/web/browse/views/directory.py

Loading...
View Options

swh/web/tests/browse/views/test_directory.py

Loading...
About · Developer information · Legal