Paths

Table of Contentst

-
swh/web/
-
web/
-
browse/views/
-
views/
7/7
origin.py
10/10
snapshot.py
-
tests/browse/views/
-
browse/
-
views/
3/3
test_origin.py

Differential D6441

URLs
ClosedPublic
Actions

Authored by jayeshv on Oct 8 2021, 12:33 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T3608: Deprecate most of the /browse/origin/.* URLs

Commits

rDWAPPS0c04ede6c05a: Deprecate /origin/log route

Summary

This fix deprecates and redirects /origin/log?origin_url=<url> and

/origin/<url>/log to /snapshot/log/
- Generic function to redirect to a new route.
- Generic logic to forward url args and query params.
- Initial tests for snapshot view

Related to T3608

Diff Detail

Repository

rDWAPPS Web applications

Branch

directory_route

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 24310
Build 37940: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 37939: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

jayeshv requested review of this revision.Oct 8 2021, 12:54 PM

WIP: Many URLs defined in the origin view (urlconf) are redundant.
This fix redirects /origin/branches?origin_url=<url> and
/origin/<url>/branches to /snapshot-branches/

Diff for log routes

Build has FAILED

Patch application report for D6441 (id=23417)

Rebasing onto 264c0bc84b...

Current branch diff-target is up to date.

Changes applied before test

commit 3bbb4320b179ab47b453e26a36f3b290c5ee57f4
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Oct 8 16:19:35 2021 +0200

    test-changes

commit f5b3cd19b787479ad090eba7f6db824783ed0978
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Oct 8 16:01:40 2021 +0200

    branch routes

commit 3c2da2ad33cf614c5e014a61457a4fed56554bd6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Oct 8 12:39:33 2021 +0200

    Deprecate most of the /browse/origin/.* URLs
    
    Many URLs defined in the origin view (urlconf) are redundant.
    This fix redirects /origin/logs?origin_url=<url> and
    /origin/<url>/log to /snapshot-log/
    - Generic decorator to deprecate a route
    - Generic logic to forward url args and query params.
    
    Related to T3608

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

Harbormaster failed remote builds in B24333: Diff 23417!Oct 8 2021, 4:44 PM

Build is green

Patch application report for D6441 (id=23418)

Rebasing onto 264c0bc84b...

Current branch diff-target is up to date.

Changes applied before test

commit 3c2da2ad33cf614c5e014a61457a4fed56554bd6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Oct 8 12:39:33 2021 +0200

    Deprecate most of the /browse/origin/.* URLs
    
    Many URLs defined in the origin view (urlconf) are redundant.
    This fix redirects /origin/logs?origin_url=<url> and
    /origin/<url>/log to /snapshot-log/
    - Generic decorator to deprecate a route
    - Generic logic to forward url args and query params.
    
    Related to T3608

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

Harbormaster completed remote builds in B24334: Diff 23418.Oct 8 2021, 4:46 PM

I added some inline comments to handle.

Also we should remove the use of /browse/origin/log/ URLs in codebase, grep for "browse-origin-log" to find them.

swh/web/browse/views/origin.py
25–46	Could you remove the decorator wrapping and simply call `redirect_to_new_route` in views implementation ? It feels weird to have functions without any instructions in them.
swh/web/browse/views/snapshot.py
118	Please use `r"snapshot/log/` instead.
129	`if snasphot_id is None`
135	`origin_url` and `timestamp` query parameters must be passed to `browse_snapshot_log` to ensure same behavior as before.
swh/web/tests/browse/views/test_origin.py
864–878	Please move the redirection check in a separate test.

This revision now requires changes to proceed.Oct 11 2021, 1:22 PM

jayeshv added inline comments.Oct 11 2021, 1:38 PM

swh/web/browse/views/origin.py
25–46	Ok, do you think adding an extra line in the doc string in each deprecated function would be enough?. It would be like "This route is deprecated, use <alternate name> instead" ?

anlambert added inline comments.Oct 11 2021, 1:44 PM

swh/web/browse/views/origin.py
25–46	Yes this should be enough, you can reference the correct view to use in sphinx using `:http:get:<url>`, see how its is done for the vault.

jayeshv added inline comments.Oct 11 2021, 1:47 PM

swh/web/browse/views/origin.py
25–46	Ok, Thanks. In fact, I have that doc string added in the coming diffs (for branches and releases). So, I will move that change to this diff and keep the decorator

Review changes

Build is green

Patch application report for D6441 (id=23441)

Rebasing onto dc489d1df2...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit b169d9dffbe99d0585d1bd2c965ba5ddc33c86d9
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 14:56:52 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic decorator to deprecate a route
    - Generic logic to forward url args and query params.
    
    Related to T3608

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

Harbormaster completed remote builds in B24357: Diff 23441.Oct 11 2021, 3:17 PM

jayeshv marked 4 inline comments as done.Oct 11 2021, 3:21 PM

jayeshv added inline comments.Oct 11 2021, 3:44 PM

swh/web/tests/browse/views/test_origin.py
884	This should be assert "/snapshot/{browse_context}/" in resp.url I will fix this with branch/release diff

It seems that the browse-snapshot-log view is missing tests so we should create the file swh/web/tests/browse/views/test_snapshot.py
and implement tests here.

Endpoint documentation must also be updated:

/browse/origin/directory/ must be marked as deprecated in docs/uri-scheme-browse-origin.rst
/browse/snapshot/log/ documentation must be added in docs/uri-scheme-browse-snapshot.rst indicating the origin_url query parameter is mandatory and that a redirection will be performed to browse the log from the latest snapshot of the origin.

swh/web/browse/views/snapshot.py
25	If `origin_url` is not provided, `get_snapshot_context` will fail so you should raise a `BadInputExc` in that case to get a 400 error page.
136	Could you do a redirection to the same view with the snapshot argument instead ? This way, the latest snapshot id will be displayed in browser URL bar.
swh/web/tests/browse/views/test_origin.py
879–884	This test is not complete and can be improved, use this instead: @given(origin()) @pytest.mark.parametrize("browse_context", ["log"]) def test_origin_view_redirects(client, browse_context, origin): query_params = {"origin_url": origin["url"]} url = reverse(f"browse-origin-{browse_context}", query_params=query_params) resp = check_html_get_response(client, url, status_code=301) assert resp["location"] == reverse( f"browse-snapshot-{browse_context}", query_params=query_params )

This revision now requires changes to proceed.Oct 11 2021, 3:47 PM

anlambert retitled this revision from Deprecate most of the /browse/origin/.* URLs to Deprecate /browse/origin/log/ URLs.Oct 11 2021, 3:48 PM

Review changes

checking for origin_url in request params
better tests
redirect to /snapshot/log/<id>

Build is green

Patch application report for D6441 (id=23447)

Rebasing onto d7c16735a4...

Current branch diff-target is up to date.

Changes applied before test

commit 47f58ef7fcc396761e6d08e784351356f51a5f22
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Oct 11 16:10:53 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic decorator to deprecate a route
    - Generic logic to forward url args and query params.
    
    Related to T3608

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

Harbormaster completed remote builds in B24364: Diff 23447.Oct 11 2021, 4:26 PM

jayeshv edited the summary of this revision. (Show Details)Oct 11 2021, 4:48 PM

jayeshv marked 5 inline comments as done.Oct 12 2021, 10:25 AM

Still some work to do before I can accept the diff:

add missing tests
remove the use of the browse-origin-log view in codebase (grep for it)

swh/web/browse/views/origin.py
25–31	I think it will be better to add a `view_redirect` parameter to the decorator instead of storing the `deprecation_mapping` dict in it. It will make the code more readable and the decorator more generic.
swh/web/browse/views/snapshot.py
21	`request.GET` is already a dictionary so that instruction is not needed.pt
26	We need a test for this.
123–136	That view is not covered by tests, we must add some in a new `tests/browse/views/test_snapshot.py` file.

This revision now requires changes to proceed.Oct 12 2021, 10:50 AM

Changes requested in the review

Removed the decorator for a redirect function
Added tests for snapshot view

jayeshv edited the summary of this revision. (Show Details)Oct 21 2021, 3:10 PM

Build is green

Patch application report for D6441 (id=23739)

Rebasing onto a8cfc14b4a...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit 116a4aae0b8115d7f86b4c0717dd47a5a37b8c5a
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Oct 21 14:54:22 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    
    Related to T3608

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

Harbormaster completed remote builds in B24627: Diff 23739.Oct 21 2021, 3:18 PM

jayeshv marked 4 inline comments as done.Oct 21 2021, 3:20 PM

Removed unused code

This is moving forward, great ! I added a couple of inline comments to handle.

Also could you add documentation for the new /browse/snapshot/log/ endpoint here
and mark the /browse/origin/logas deprecated (see example here) ?

swh/web/browse/views/snapshot.py
26	This call will fail if the `request.GET` dict contains parameters unexpected by the `get_snapshot_context` function. You should explicitly provide the `origin_url` and `timestamp` parameter instead.
swh/web/tests/browse/views/test_snapshot.py
19 ↗	(On Diff #23739)	Use this instead: from swh.web.tests.django_asserts import assert_contains assert_contains(resp, f"swh:1:snp:{snapshot}") Same thing for other tests.

This revision now requires changes to proceed.Oct 21 2021, 4:28 PM

Build is green

Patch application report for D6441 (id=23745)

Rebasing onto a8cfc14b4a...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit a006ec12220115be99f794dbdd24214462660987
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Oct 21 16:16:07 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    
    Related to T3608

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

Harbormaster completed remote builds in B24632: Diff 23745.Oct 21 2021, 4:32 PM

Requested changes

jayeshv marked 2 inline comments as done.Oct 21 2021, 4:45 PM

Build is green

Patch application report for D6441 (id=23749)

Rebasing onto a8cfc14b4a...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit aa9ed3401503f2093334c1e27c92a503f807a2be
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Oct 21 16:43:19 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    
    Related to T3608

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

Harbormaster completed remote builds in B24636: Diff 23749.Oct 21 2021, 4:59 PM

Documentation updates

Build was aborted

Patch application report for D6441 (id=23755)

Rebasing onto a8cfc14b4a...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit 60dd6faae996e38b67109d37daac4b9617b2ae46
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Oct 21 17:46:53 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    - Documentation updates
    
    Related to T3608

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

Harbormaster failed remote builds in B24642: Diff 23755!Oct 21 2021, 6:12 PM

Build is green

Patch application report for D6441 (id=23755)

Rebasing onto a8cfc14b4a...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/log route

Changes applied before test

commit b30080209cc0285432c1408e42686335340311c8
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Oct 21 17:46:53 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    - Documentation updates
    
    Related to T3608

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

Harbormaster completed remote builds in B24642: Diff 23755.Oct 22 2021, 9:26 AM

Looks good to me, thanks ! I added a last batch of nitpick comments to handle before landing this.

docs/uri-scheme-browse-snapshot.rst
183 ↗	(On Diff #23755)	s/origin_url/origin URL/
191 ↗	(On Diff #23755)	extra "in the" to remove
swh/web/browse/views/origin.py
142	Use http:get:`/browse/snapshot/log` to link to endpoint doc
162	same here
swh/web/browse/views/snapshot.py
127	The urls that point to it are :http:get:`/browse/snapshot/log` and

This revision is now accepted and ready to land.Oct 22 2021, 11:29 AM

typo fixes; added doc links

jayeshv marked 5 inline comments as done.Oct 22 2021, 11:53 AM

Build is green

Patch application report for D6441 (id=23766)

Rebasing onto 0da5060e6f...

Current branch diff-target is up to date.

Changes applied before test

commit 0c04ede6c05aff2dea636ad3c560acdeaf94b2cf
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Oct 22 11:45:28 2021 +0200

    Deprecate /origin/log route
    
    This fix deprecates and redirects /origin/log?origin_url=<url> and
    /origin/<url>/log to /snapshot/log/
    - Generic function to redirect to a new route.
    - Generic logic to forward url args and query params.
    - Initial tests for snapshot view.
    - Documentation updates
    
    Related to T3608

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

Harbormaster completed remote builds in B24652: Diff 23766.Oct 22 2021, 12:03 PM

Closed by commit rDWAPPS0c04ede6c05a: Deprecate /origin/log route (authored by jayeshv). · Explain WhyOct 22 2021, 12:06 PM

This revision was automatically updated to reflect the committed changes.

jayeshv added a commit: rDWAPPS0c04ede6c05a: Deprecate /origin/log route.

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

swh/

web/

browse/

views/

origin.py

31 lines

80%

92%

snapshot.py

9 lines

50%

20%

tests/

browse/

views/

test_origin.py

20 lines

98%

100%

Diff 23393

View Options

swh/web/browse/views/origin.py

# Copyright (C) 2017-2020 The Software Heritage developers		# Copyright (C) 2017-2020 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution		# See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version		# License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information		# See top-level LICENSE file for more information

		import functools

from django.shortcuts import redirect, render		from django.shortcuts import redirect, render
		from django.urls import resolve

from swh.web.browse.browseurls import browse_route		from swh.web.browse.browseurls import browse_route
from swh.web.browse.snapshot_context import (		from swh.web.browse.snapshot_context import (
browse_snapshot_branches,		browse_snapshot_branches,
browse_snapshot_content,		browse_snapshot_content,
browse_snapshot_directory,		browse_snapshot_directory,
browse_snapshot_log,		browse_snapshot_log,
browse_snapshot_releases,		browse_snapshot_releases,
get_snapshot_context,		get_snapshot_context,
)		)
from swh.web.common import archive		from swh.web.common import archive
from swh.web.common.exc import BadInputExc		from swh.web.common.exc import BadInputExc, NotFoundExc
from swh.web.common.origin_visits import get_origin_visits		from swh.web.common.origin_visits import get_origin_visits
from swh.web.common.utils import format_utc_iso_date, parse_iso8601_date_to_utc, reverse		from swh.web.common.utils import format_utc_iso_date, parse_iso8601_date_to_utc, reverse


		def deprecated_route(func):
		# A mapping to relate deprecated URls to the new ones
		deprecation_mapping = {
		# log URLs
		"browse-origin-log": "browse-snapshot-log",
		"browse-origin-log-legacy": "browse-snapshot-log",
		anlambertUnsubmitted Done Inline Actions I think it will be better to add a `view_redirect` parameter to the decorator instead of storing the `deprecation_mapping` dict in it. It will make the code more readable and the decorator more generic. anlambert: I think it will be better to add a `view_redirect` parameter to the decorator instead of…
		}

		@functools.wraps(func)
		def redirect_to_new_route(request, args, *kwargs):
		request_path = resolve(request.path_info)
		if request_path.url_name not in deprecation_mapping:
		raise NotFoundExc("Invalid deprecated route")
		# Send all the url_args and the request_args as query params
		# eg /origin/<url:url-val>/log?path=test
		# will be send as /log?url=<url-val>&path=test
		args = {request_path.kwargs, {k: v for k, v in request.GET.items()}}
		return redirect(
		reverse(deprecation_mapping[request_path.url_name], query_params=args)
		)

		anlambertUnsubmitted Done Inline Actions Could you remove the decorator wrapping and simply call `redirect_to_new_route` in views implementation ? It feels weird to have functions without any instructions in them. anlambert: Could you remove the decorator wrapping and simply call `redirect_to_new_route` in views…
		jayeshvAuthorUnsubmitted Done Inline Actions Ok, do you think adding an extra line in the doc string in each deprecated function would be enough?. It would be like "This route is deprecated, use <alternate name> instead" ? jayeshv: Ok, do you think adding an extra line in the doc string in each deprecated function would be…
		anlambertUnsubmitted Done Inline Actions Yes this should be enough, you can reference the correct view to use in sphinx using `:http:get:<url>`, see how its is done for the vault. anlambert: Yes this should be enough, you can reference the correct view to use in sphinx using `:http:get…
		jayeshvAuthorUnsubmitted Done Inline Actions Ok, Thanks. In fact, I have that doc string added in the coming diffs (for branches and releases). So, I will move that change to this diff and keep the decorator jayeshv: Ok, Thanks. In fact, I have that doc string added in the coming diffs (for branches and…
		return redirect_to_new_route


@browse_route(		@browse_route(
r"origin/directory/", view_name="browse-origin-directory",		r"origin/directory/", view_name="browse-origin-directory",
)		)
def origin_directory_browse(request):		def origin_directory_browse(request):
"""Django view for browsing the content of a directory associated		"""Django view for browsing the content of a directory associated
to an origin for a given visit.		to an origin for a given visit.

The URL that points to it is :http:get:`/browse/origin/directory/`		The URL that points to it is :http:get:`/browse/origin/directory/`
▲ Show 20 Lines • Show All 74 Lines • ▼ Show 20 Lines	return browse_snapshot_content(
path=path,		path=path,
selected_language=request.GET.get("language"),		selected_language=request.GET.get("language"),
)		)


@browse_route(		@browse_route(
r"origin/log/", view_name="browse-origin-log",		r"origin/log/", view_name="browse-origin-log",
)		)
		@deprecated_route
def origin_log_browse(request):		def origin_log_browse(request):
"""Django view that produces an HTML display of revisions history (aka		"""Django view that produces an HTML display of revisions history (aka
		anlambertUnsubmitted Done Inline Actions Use http:get:`/browse/snapshot/log` to link to endpoint doc anlambert: %%%Use http:get:`/browse/snapshot/log` to link to endpoint doc %%%
the commit log) associated to a software origin.		the commit log) associated to a software origin.

The URL that points to it is :http:get:`/browse/origin/log/`		The URL that points to it is :http:get:`/browse/origin/log/`
"""		"""
return browse_snapshot_log(		return browse_snapshot_log(
request,		request,
origin_url=request.GET.get("origin_url"),		origin_url=request.GET.get("origin_url"),
snapshot_id=request.GET.get("snapshot"),		snapshot_id=request.GET.get("snapshot"),
timestamp=request.GET.get("timestamp"),		timestamp=request.GET.get("timestamp"),
)		)


@browse_route(		@browse_route(
r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/log/",		r"origin/(?P<origin_url>.+)/visit/(?P<timestamp>.+)/log/",
r"origin/(?P<origin_url>.+)/log/",		r"origin/(?P<origin_url>.+)/log/",
view_name="browse-origin-log-legacy",		view_name="browse-origin-log-legacy",
)		)
		@deprecated_route
def origin_log_browse_legacy(request, origin_url, timestamp=None):		def origin_log_browse_legacy(request, origin_url, timestamp=None):
"""Django view that produces an HTML display of revisions history (aka		"""Django view that produces an HTML display of revisions history (aka
		anlambertUnsubmitted Done Inline Actions same here anlambert: same here
the commit log) associated to a software origin.		the commit log) associated to a software origin.

The URLs that point to it are		The URLs that point to it are
:http:get:`/browse/origin/(origin_url)/log/` and		:http:get:`/browse/origin/(origin_url)/log/` and
:http:get:`/browse/origin/(origin_url)/visit/(timestamp)/log/`		:http:get:`/browse/origin/(origin_url)/visit/(timestamp)/log/`

"""		"""
return browse_snapshot_log(		return browse_snapshot_log(
▲ Show 20 Lines • Show All 176 Lines • Show Last 20 Lines

View Options

swh/web/browse/views/snapshot.py

# Copyright (C) 2018-2019 The Software Heritage developers		# Copyright (C) 2018-2019 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution		# See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version		# License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information		# See top-level LICENSE file for more information


from django.shortcuts import redirect		from django.shortcuts import redirect

from swh.web.browse.browseurls import browse_route		from swh.web.browse.browseurls import browse_route
from swh.web.browse.snapshot_context import (		from swh.web.browse.snapshot_context import (
browse_snapshot_branches,		browse_snapshot_branches,
browse_snapshot_content,		browse_snapshot_content,
browse_snapshot_directory,		browse_snapshot_directory,
browse_snapshot_log,		browse_snapshot_log,
browse_snapshot_releases,		browse_snapshot_releases,
		get_snapshot_context,
)		)
from swh.web.common.utils import reverse		from swh.web.common.utils import reverse


@browse_route(		@browse_route(
		anlambertUnsubmitted Done Inline Actions `request.GET` is already a dictionary so that instruction is not needed.pt anlambert: `request.GET` is already a [[ https://docs.djangoproject.com/fr/3.2/ref/request…
r"snapshot/(?P<snapshot_id>[0-9a-f]+)/",		r"snapshot/(?P<snapshot_id>[0-9a-f]+)/",
view_name="browse-snapshot",		view_name="browse-snapshot",
checksum_args=["snapshot_id"],		checksum_args=["snapshot_id"],
)		)
		anlambertUnsubmitted Done Inline Actions If `origin_url` is not provided, `get_snapshot_context` will fail so you should raise a `BadInputExc` in that case to get a 400 error page. anlambert: If `origin_url` is not provided, `get_snapshot_context` will fail so you should raise a…
def snapshot_browse(request, snapshot_id):		def snapshot_browse(request, snapshot_id):
		anlambertUnsubmitted Done Inline Actions We need a test for this. anlambert: We need a test for this.
		anlambertUnsubmitted Done Inline Actions This call will fail if the `request.GET` dict contains parameters unexpected by the `get_snapshot_context` function. You should explicitly provide the `origin_url` and `timestamp` parameter instead. anlambert: This call will fail if the `request.GET` dict contains parameters unexpected by the…
"""Django view for browsing the content of a snapshot.		"""Django view for browsing the content of a snapshot.

The url that points to it is :http:get:`/browse/snapshot/(snapshot_id)/`		The url that points to it is :http:get:`/browse/snapshot/(snapshot_id)/`
"""		"""
browse_snapshot_url = reverse(		browse_snapshot_url = reverse(
"browse-snapshot-directory",		"browse-snapshot-directory",
url_args={"snapshot_id": snapshot_id},		url_args={"snapshot_id": snapshot_id},
query_params=request.GET,		query_params=request.GET,
▲ Show 20 Lines • Show All 75 Lines • ▼ Show 20 Lines	return browse_snapshot_content(
request,		request,
snapshot_id=snapshot_id,		snapshot_id=snapshot_id,
path=path,		path=path,
selected_language=request.GET.get("language"),		selected_language=request.GET.get("language"),
)		)


@browse_route(		@browse_route(
r"snapshot/(?P<snapshot_id>[0-9a-f]+)/log/",		r"snapshot/(?P<snapshot_id>[0-9a-f]+)/log/",
		anlambertUnsubmitted Done Inline Actions Please use `r"snapshot/log/` instead. anlambert: Please use `r"snapshot/log/` instead.
		r"snapshot-log/",
view_name="browse-snapshot-log",		view_name="browse-snapshot-log",
checksum_args=["snapshot_id"],		checksum_args=["snapshot_id"],
)		)
def snapshot_log_browse(request, snapshot_id):		def snapshot_log_browse(request, snapshot_id=None):
"""Django view that produces an HTML display of revisions history (aka		"""Django view that produces an HTML display of revisions history (aka
the commit log) collected in a snapshot.		the commit log) collected in a snapshot.

The url that points to it is		The url that points to it is
		anlambertUnsubmitted Done Inline Actions The urls that point to it are :http:get:`/browse/snapshot/log` and anlambert: %%%The urls that point to it are :http:get:`/browse/snapshot/log` and %%%
:http:get:`/browse/snapshot/(snapshot_id)/log/`		:http:get:`/browse/snapshot/(snapshot_id)/log/`
"""		"""
		anlambertUnsubmitted Done Inline Actions `if snasphot_id is None` anlambert: `if snasphot_id is None`
		if not snapshot_id:
		# This case happens when redirected from /origin/log
		args = {key: value for key, value in request.GET.items()}
		snapshot_context = get_snapshot_context(**args)
		snapshot_id = snapshot_context["snapshot_id"]
return browse_snapshot_log(request, snapshot_id=snapshot_id)		return browse_snapshot_log(request, snapshot_id=snapshot_id)
		anlambertUnsubmitted Done Inline Actions `origin_url` and `timestamp` query parameters must be passed to `browse_snapshot_log` to ensure same behavior as before. anlambert: `origin_url` and `timestamp` query parameters must be passed to `browse_snapshot_log` to ensure…

		anlambertUnsubmitted Done Inline Actions Could you do a redirection to the same view with the snapshot argument instead ? This way, the latest snapshot id will be displayed in browser URL bar. anlambert: Could you do a redirection to the same view with the snapshot argument instead ? This way, the…
		anlambertUnsubmitted Done Inline Actions That view is not covered by tests, we must add some in a new `tests/browse/views/test_snapshot.py` file. anlambert: That view is not covered by tests, we must add some in a new `tests/browse/views/test_snapshot.

@browse_route(		@browse_route(
r"snapshot/(?P<snapshot_id>[0-9a-f]+)/branches/",		r"snapshot/(?P<snapshot_id>[0-9a-f]+)/branches/",
view_name="browse-snapshot-branches",		view_name="browse-snapshot-branches",
checksum_args=["snapshot_id"],		checksum_args=["snapshot_id"],
)		)
def snapshot_branches_browse(request, snapshot_id):		def snapshot_branches_browse(request, snapshot_id):
"""Django view that produces an HTML display of the list of releases		"""Django view that produces an HTML display of the list of releases
Show All 21 Lines

View Options

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

Show First 20 Lines • Show All 855 Lines • ▼ Show 20 Lines	for browse_context in (
"directory",		"directory",
"log",		"log",
"branches",		"branches",
"releases",		"releases",
"visits",		"visits",
):		):
url = reverse(f"browse-origin-{browse_context}")		url = reverse(f"browse-origin-{browse_context}")

		if browse_context == "log":
		# FIXME; temporory fix for partial T3608
		# Will be removed once every origin duplicate URL is moved
		resp = check_html_get_response(client, url, status_code=302)
		assert "/snapshot-log/" in resp.url
		else:
resp = check_html_get_response(		resp = check_html_get_response(
client, url, status_code=400, template_used="error.html"		client, url, status_code=400, template_used="error.html"
)		)
assert_contains(		assert_contains(
resp, "An origin URL must be provided as query parameter.", status_code=400		resp,
		"An origin URL must be provided as query parameter.",
		status_code=400,
)		)

		anlambertUnsubmitted Done Inline Actions Please move the redirection check in a separate test. anlambert: Please move the redirection check in a separate test.

def _origin_content_view_test_helper(		def _origin_content_view_test_helper(
client,		client,
archive_data,		archive_data,
origin_info,		origin_info,
origin_visit,		origin_visit,
		anlambertUnsubmitted Done Inline Actions This test is not complete and can be improved, use this instead: @given(origin()) @pytest.mark.parametrize("browse_context", ["log"]) def test_origin_view_redirects(client, browse_context, origin): query_params = {"origin_url": origin["url"]} url = reverse(f"browse-origin-{browse_context}", query_params=query_params) resp = check_html_get_response(client, url, status_code=301) assert resp["location"] == reverse( f"browse-snapshot-{browse_context}", query_params=query_params ) anlambert: This test is not complete and can be improved, use this instead: ```lang=python @given(origin…
		jayeshvAuthorUnsubmitted Done Inline Actions This should be assert "/snapshot/{browse_context}/" in resp.url I will fix this with branch/release diff jayeshv: This should be assert "/snapshot/{browse_context}/" in resp.url I will fix this with…
snapshot_sizes,		snapshot_sizes,
origin_branches,		origin_branches,
origin_releases,		origin_releases,
root_dir_sha1,		root_dir_sha1,
content,		content,
visit_id=None,		visit_id=None,
timestamp=None,		timestamp=None,
snapshot_id=None,		snapshot_id=None,
▲ Show 20 Lines • Show All 452 Lines • Show Last 20 Lines

Deprecate /browse/origin/log/ URLsClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D6441 (id=23417)

Changes applied before test

Patch application report for D6441 (id=23418)

Changes applied before test

Patch application report for D6441 (id=23441)

Changes applied before test

Patch application report for D6441 (id=23447)

Changes applied before test

Patch application report for D6441 (id=23739)

Changes applied before test

Patch application report for D6441 (id=23745)

Changes applied before test

Patch application report for D6441 (id=23749)

Changes applied before test

Patch application report for D6441 (id=23755)

Changes applied before test

Patch application report for D6441 (id=23755)

Changes applied before test

Patch application report for D6441 (id=23766)

Changes applied before test

Revision ContentsChangeset List

Diff 23393

swh/web/browse/views/origin.py

swh/web/browse/views/snapshot.py

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

Deprecate /browse/origin/log/ URLs
ClosedPublic
Actions

Revision Contents
Changeset List