Page MenuHomeSoftware Heritage

Adding 'Add Origin info' checkbox which was missing
ClosedPublic

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

Details

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

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

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

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

  • Adding 'Add Origin info' checkbox which was missing
  • Adding tests to check permalink-box content
vlorentz requested changes to this revision.Feb 17 2020, 1:15 PM

Thanks!

Just one comment:

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

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
63–69

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">')
74

I agree with @vlorentz

  • removed hardcoded data and updated test_directory.py as asked
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

rebase to origin/master and squash commits

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