Page MenuHomeSoftware Heritage
Differential D3042

templates/directory-display: Display message when directory/revision is missing
ClosedPublic
Actions

Authored by anlambert on Apr 21 2020, 7:01 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2353: Improve snapshot browsing when error occurs
Commits
rDWAPPS09e18940394e: templates/directory-display: Display message for missing directory/revision
Summary

That message can only be displayed when browsing a directory in a snapshot
context, for instance when browsing a branch whith a non resolvable revision.

I picked a quite generic error message, maybe it could be improved to mention
the revision is not resolvable ?

Related to T2353

Diff Detail

Repository
rDWAPPS Web applications
Branch
missing-directory-improve-displayed-message
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11961
Build 18133: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 18132: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
0 msJenkins > should have less width when collapsed compared to open::Tests / Cypress tests / Run cypress tests / Sidebar tests On Large Screen should have less width when collapsed compared to open
AssertionError: expected 250 to be below 250 at Context.<anonymous> (http://localhost:5004/__cypress/tests?p=cypress/integration/sidebar.spec.js-441:45:20)
554 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_accept_pending_save_request
1,138 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_authorized_origin_url
177 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_unauthorized_origin_url
194 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_reject_pending_save_request
View Full Test Results (1 Failed · 498 Passed · 7 Skipped)

Event Timeline

anlambert created this revision.Apr 21 2020, 7:01 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 21 2020, 7:01 PM
anlambert mentioned this in T2353: Improve snapshot browsing when error occurs.Apr 21 2020, 7:03 PM
ardumont accepted this revision.Apr 21 2020, 7:07 PM
ardumont added a subscriber: ardumont.

I picked a quite generic error message, maybe it could be improved to mention
the revision is not resolvable ?

Can we, at the moment, discriminate between an unresolvable revision and an unresolvable directory ?

If we can, adding the switch case here would make sense, thus we'd have the means to have different messages.

Otherwise, that's fine ;)

This revision is now accepted and ready to land.Apr 21 2020, 7:07 PM
swh-public-ci added a comment.Apr 21 2020, 7:10 PM

Build is green

Patch application report for D3042 (id=10810)

Rebasing onto 40ac1374bc...

Current branch diff-target is up to date.
Changes applied before test
commit a0a74ddc95fd9ff63821b6a481ec51ebef210477
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 21 18:55:10 2020 +0200

    templates/directory-display: Display message when directory is missing
    
    That message can only be displayed when browsing a directory in a snapshot
    context, for instance when browsing a branch whith a non resolvable revision.
    
    Related to T2353

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

Harbormaster completed remote builds in B11960: Diff 10810.Apr 21 2020, 7:10 PM
anlambert updated this revision to Diff 10811.Apr 21 2020, 7:29 PM

Update: Improve error message for missing revision

anlambert retitled this revision from templates/directory-display: Display message when directory is missing to templates/directory-display: Display message when directory/revision is missing.Apr 21 2020, 7:30 PM

I added an extra check for the revision if the directory is None in order to improve error message.

swh-public-ci added a comment.Apr 21 2020, 7:38 PM

Build has FAILED

Patch application report for D3042 (id=10811)

Rebasing onto 40ac1374bc...

Current branch diff-target is up to date.
Changes applied before test
commit eb992431b1544a02e3dc1bf257aec0fa526c4595
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 21 18:55:10 2020 +0200

    templates/directory-display: Display message for missing directory/revision
    
    That message can only be displayed when browsing a directory in a snapshot
    context, for instance when browsing a branch whith a non resolvable revision.
    
    Related to T2353

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

Harbormaster failed remote builds in B11961: Diff 10811!Apr 21 2020, 7:38 PM
ardumont added a comment.Apr 22 2020, 9:25 AM

Yes, even better now.

(we are missing the test case about revision ok, revision.directory not found though)

anlambert added a comment.Apr 22 2020, 11:39 AM

we are missing the test case about revision ok

This is already implemented in test_origin_*_directory_view test cases (checking browsing a directory in an origin context is OK).

revision.directory not found though)

I should be able to work something out for that one.

ardumont added a comment.Apr 22 2020, 11:48 AM

revision.directory not found though)

I should be able to work something out for that one.

Yes, i meant only the 1 test case ;)

anlambert added a comment.Apr 22 2020, 12:05 PM

I should be able to work something out for that one.

Actually, it is already tested in test_browse_origin_directory_not_found.

There is already a 404 error page displayed when a directory in an origin context could not be found in the archive.

So I will only scope that diff for the branch with non resolvable revision case.

The way the browsing errors are displayed in a snapshot context still need to be improved though.

ardumont added a comment.Apr 22 2020, 12:06 PM

Ack

heads up, i had to clean up the staging infra so don't worry if you don't find back the data there.

context is T2371#43636

anlambert added a comment.Apr 22 2020, 12:09 PM

Ack, I will land the diff and push a new swh-web tag.

I will get back on T2353 once I finished to implement T2342.

ardumont accepted this revision.Apr 22 2020, 12:18 PM

Yes, please.

And thanks again ;)

anlambert updated this revision to Diff 10813.Apr 22 2020, 12:20 PM

Update: Remove redundant error message in template and prevent django.template.base.VariableDoesNotExist exception

swh-public-ci added a comment.Apr 22 2020, 12:28 PM

Build has FAILED

Patch application report for D3042 (id=10813)

Rebasing onto 40ac1374bc...

Current branch diff-target is up to date.
Changes applied before test
commit a150993fac873cc7faaf4100dd918605c02af32e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 21 18:55:10 2020 +0200

    templates/directory-display: Display message for missing directory/revision
    
    That message can only be displayed when browsing a directory in a snapshot
    context, for instance when browsing a branch whith a non resolvable revision.
    
    Related to T2353

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

Harbormaster failed remote builds in B11965: Diff 10813!Apr 22 2020, 12:28 PM
anlambert updated this revision to Diff 10814.Apr 22 2020, 12:38 PM

Update: Fix test syntax in django template (no line break allowed, sigh ...)

swh-public-ci added a comment.Apr 22 2020, 12:46 PM

Build is green

Patch application report for D3042 (id=10814)

Rebasing onto 40ac1374bc...

Current branch diff-target is up to date.
Changes applied before test
commit 09e18940394e6b676b245017c2507f8b9b0638b3
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 21 18:55:10 2020 +0200

    templates/directory-display: Display message for missing directory/revision
    
    That message can only be displayed when browsing a directory in a snapshot
    context, for instance when browsing a branch whith a non resolvable revision.
    
    Related to T2353

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

Harbormaster completed remote builds in B11966: Diff 10814.Apr 22 2020, 12:46 PM
Closed by commit rDWAPPS09e18940394e: templates/directory-display: Display message for missing directory/revision (authored by anlambert). · Explain WhyApr 22 2020, 12:54 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPS09e18940394e: templates/directory-display: Display message for missing directory/revision.

Revision Contents
Changeset List

PathSize
swh/
web/
browse/
8 lines
templates/
includes/
6 lines
tests/
browse/
views/
37 lines

Diff 10811

View Options

swh/web/browse/snapshot_context.py

Loading...
View Options

swh/web/templates/includes/directory-display.html

Loading...
View Options

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

Loading...
About · Developer information · Legal