Page MenuHomeSoftware Heritage

browse/revision: Fix revision id in snapshot context
ClosedPublic

Authored by anlambert on Jun 4 2020, 3:42 PM.

Details

Summary

Revision at the tip of the current branch was used as anchor instead of
the current browsed one.

Closes T2417

Depends on D3156.

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

anlambert created this revision.Jun 4 2020, 3:42 PM

Build is green

Patch application report for D3223 (id=11433)

Could not rebase; Attempt merge onto 3f9f08482c...

Updating 3f9f0848..312db807
Fast-forward
 swh/web/browse/snapshot_context.py           |  44 ++---
 swh/web/browse/utils.py                      |  41 +++--
 swh/web/browse/views/content.py              |  65 ++++---
 swh/web/browse/views/directory.py            |  69 +++++---
 swh/web/browse/views/origin.py               |  33 +++-
 swh/web/browse/views/release.py              |  21 ++-
 swh/web/browse/views/revision.py             |  66 +++++---
 swh/web/browse/views/snapshot.py             |   2 +-
 swh/web/tests/browse/views/test_content.py   | 109 ++++++++++++
 swh/web/tests/browse/views/test_directory.py | 127 +++++++++++++-
 swh/web/tests/browse/views/test_origin.py    | 136 ++++++++++++---
 swh/web/tests/browse/views/test_release.py   |  83 +++++----
 swh/web/tests/browse/views/test_revision.py  | 244 +++++++++++++++------------
 13 files changed, 750 insertions(+), 290 deletions(-)
Changes applied before test
commit 312db80743a7961b70cd31d581543e24eed4769b
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon May 25 15:45:10 2020 +0200

    browse/revision: Fix revision id in snapshot context
    
    Closes T2417

commit 1d477dface8adc18ddf37ac48e370b7fdcd5426a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Fri Apr 3 19:40:04 2020 +0200

    browse: Add snapshot related query parameters in numerous object views
    
    It enables to browse archived objects scoped to a given origin visit, possibly
    on a given branch, release or revision.
    
    Related to T2342
    Related to T2427
    
    Closes T2359

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

I don't quite follow the fix (must be tired ¯\_(ツ)_/¯)

I don't quite follow the fix (must be tired ¯\_(ツ)_/¯)

The task created by @rdicosmo (T2417) perfectly explains the fixed issue ;-)

I don't quite follow the fix (must be tired ¯\_(ツ)_/¯)

The task created by @rdicosmo (T2417) perfectly explains the fixed issue ;-)

sure, it's not the task i don't follow, it's the code

ardumont accepted this revision.Jun 5 2020, 9:40 AM

I guess it's ok.

This revision is now accepted and ready to land.Jun 5 2020, 9:40 AM

A quick comment on the code above: it seems to depend on the use of sha1_git parameters passed in the urls used for browsing.
While this is perfectly ok for today, we need a way to be future proof, when different versions of SWHIDs will come in, using different hashing algorithms.
Since we commit to maintain forever the resolution of previous versions of identifiers, the navigation in the webapp will need to be able to accomodate multiple hashing algorithm at the same time, and we need to plan to structure the code accordingly.

@anlambert : if you agree with the above, may you turn this comment into a task of its own?

A quick comment on the code above: it seems to depend on the use of sha1_git parameters passed in the urls used for browsing.
While this is perfectly ok for today, we need a way to be future proof, when different versions of SWHIDs will come in, using different hashing algorithms.
Since we commit to maintain forever the resolution of previous versions of identifiers, the navigation in the webapp will need to be able to accomodate multiple hashing algorithm at the same time, and we need to plan to structure the code accordingly.

@anlambert : if you agree with the above, may you turn this comment into a task of its own?

@rdicosmo , I got your point. I have created T2435 on the subject.