Page MenuHomeSoftware Heritage

Navigation to directory(code) is broken in snapshot view
ClosedPublic

Authored by jayeshv on Mon, Oct 11, 1:05 PM.

Details

Summary

URL to the code (directory) view was missing in case the snapshot is
missing an origin cotext. Added an extra parameter called directory_url in
the snapshot_context to fix this.

realated to T3644

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

Build is green

Patch application report for D6449 (id=23429)

Rebasing onto 264c0bc84b...

Current branch diff-target is up to date.
Changes applied before test
commit 0616a1711d235135afc66a972f42bca71903851f
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 12:35:02 2021 +0200

    Navigation to directory(code) is broken in snapshot view
    
    URL to the code (directory) view was missing in case the snapshot is
    missing an origin cotext. Added an extra parameter called directory_url in
    the snapshot_context to fix this.
    
    realated to T3644

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

anlambert added a subscriber: anlambert.

Looks good but some small changes are required.

swh/web/browse/snapshot_context.py
483

There is still some use of visit_info.url in a template so you should rather do
visit_info["url"] = directory_url = visit_url

swh/web/tests/browse/test_snapshot_context.py
234

same here

This revision now requires changes to proceed.Mon, Oct 11, 1:33 PM

Please use the imperative form in diff and commit titles. For example (respectively):

Fix navigation to directory(code) in snapshot view

and

Add an extra parameter called directory_url in the snapshot_context

To fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

or just this:

Fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

Two other nitpicks:

  1. it's easier for everyone to have the diff title and message match the commit message most of the time
  2. Use "Fixes Txxxx" or "Closes Txxxx" instead of "Related to Txxxx" for commits that completely fix an issue, it gives more details on the relation between the commit and the task.

See https://docs.softwareheritage.org/devel/contributing/git-style-guide.html

Please use the imperative form in diff and commit titles. For example (respectively):

Fix navigation to directory(code) in snapshot view

and

Add an extra parameter called directory_url in the snapshot_context

To fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

or just this:

Fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

Two other nitpicks:

  1. it's easier for everyone to have the diff title and message match the commit message most of the time
  2. Use "Fixes Txxxx" or "Closes Txxxx" instead of "Related to Txxxx" for commits that completely fix an issue, it gives more details on the relation between the commit and the task.

See https://docs.softwareheritage.org/devel/contributing/git-style-guide.html

Please use the imperative form in diff and commit titles. For example (respectively):

Fix navigation to directory(code) in snapshot view

and

Add an extra parameter called directory_url in the snapshot_context

To fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

or just this:

Fix missing URL to the code (directory) view in case the snapshot is missing an origin context.

Related to T3644

Two other nitpicks:

  1. it's easier for everyone to have the diff title and message match the commit message most of the time
  2. Use "Fixes Txxxx" or "Closes Txxxx" instead of "Related to Txxxx" for commits that completely fix an issue, it gives more details on the relation between the commit and the task.

See https://docs.softwareheritage.org/devel/contributing/git-style-guide.html

Thanks. I will address these comments as well.

anlambert requested changes to this revision.EditedMon, Oct 11, 2:17 PM

Looks good for the code but still some issues in the commit message (typo + formatting),
I would have done like this for instance.

browse/snapshot_context: Fix missing code URL in some snapshot views

URL to the code (directory) view was missing when the snapshot is missing an origin context. 

Add an extra field called directory_url in the snapshot context to fix this.

Related to T3644
This revision now requires changes to proceed.Mon, Oct 11, 2:17 PM

Build was aborted

Patch application report for D6449 (id=23434)

Rebasing onto dc489d1df2...

First, rewinding head to replay your work on top of it...
Applying: URL to the code (directory) view was missing in case the snapshot is
Changes applied before test
commit d7edf052645d36831d198f2037530c57d2713838
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 13:57:16 2021 +0200

    URL to the code (directory) view was missing in case the snapshot is
    missing an origin cotext. Added an extra parameter called directory_url in
    the snapshot_context to fix this.
    
    realated to T3644

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

Review changes and changes to the commit message

Commit message changes to include "Fixes <ticket>"

Build is green

Patch application report for D6449 (id=23442)

Rebasing onto dc489d1df2...

Current branch diff-target is up to date.
Changes applied before test
commit 4284400015d1454284e41c40967acf3e80ac3dbf
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 15:07:24 2021 +0200

    Fix navigation to directory(code) in snapshot view.
    
    Add an extra parameter called directory_url in the snapshot_context.
    To fix the missing URL to the code (directory) view in case the
    snapshot is missing an origin context.
    
    Related to T3644

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

Looks good to me, thanks !

This revision is now accepted and ready to land.Mon, Oct 11, 3:35 PM

Build is green

Patch application report for D6449 (id=23443)

Rebasing onto dc489d1df2...

Current branch diff-target is up to date.
Changes applied before test
commit d7c16735a46d0ea78f0fa48b16f66dc30bab006d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 15:19:26 2021 +0200

    Fix navigation to directory(code) in snapshot view.
    
    Add an extra parameter called directory_url in the snapshot_context.
    To fix the missing URL to the code (directory) view in case the
    snapshot is missing an origin context.
    
    Fixes T3644

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