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
Branch
permalink-box-context
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10630
Build 15917: Cypress tests for swh-web diffsJenkins
Build 15916: tox-on-jenkinsJenkins
Build 15915: arc lint + arc unit

Event Timeline

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

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 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

  • removed hardcoded data and updated test_directory.py as asked

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

This revision is now accepted and ready to land.Feb 18 2020, 4:23 PM