diff --git a/swh/web/browse/views/origin.py b/swh/web/browse/views/origin.py --- a/swh/web/browse/views/origin.py +++ b/swh/web/browse/views/origin.py @@ -3,23 +3,51 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +import functools + from django.shortcuts import redirect, render +from django.urls import resolve from swh.web.browse.browseurls import browse_route from swh.web.browse.snapshot_context import ( - browse_snapshot_branches, browse_snapshot_content, browse_snapshot_directory, - browse_snapshot_log, browse_snapshot_releases, get_snapshot_context, ) 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.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 routes + "browse-origin-log": "browse-snapshot-log", + "browse-origin-log-legacy": "browse-snapshot-log", + # branches routes + "browse-origin-branches": "browse-snapshot-branches", + "browse-origin-branches-legacy": "browse-snapshot-branches", + } + + @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//log?path=test + # will be send as /log?url=&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) + ) + + return redirect_to_new_route + + @browse_route( r"origin/directory/", view_name="browse-origin-directory", ) @@ -110,18 +138,16 @@ @browse_route( r"origin/log/", view_name="browse-origin-log", ) +@deprecated_route def origin_log_browse(request): - """Django view that produces an HTML display of revisions history (aka + """ + This route is deprecated, use /snapshot-log/ instead + + Django view that produces an HTML display of revisions history (aka the commit log) associated to a software origin. The URL that points to it is :http:get:`/browse/origin/log/` """ - return browse_snapshot_log( - request, - origin_url=request.GET.get("origin_url"), - snapshot_id=request.GET.get("snapshot"), - timestamp=request.GET.get("timestamp"), - ) @browse_route( @@ -129,8 +155,12 @@ r"origin/(?P.+)/log/", view_name="browse-origin-log-legacy", ) +@deprecated_route def origin_log_browse_legacy(request, origin_url, timestamp=None): - """Django view that produces an HTML display of revisions history (aka + """ + This route is deprecated, use /snapshot-log/ instead + + Django view that produces an HTML display of revisions history (aka the commit log) associated to a software origin. The URLs that point to it are @@ -138,31 +168,22 @@ :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/log/` """ - return browse_snapshot_log( - request, - origin_url=origin_url, - snapshot_id=request.GET.get("snapshot"), - timestamp=timestamp, - ) @browse_route( r"origin/branches/", view_name="browse-origin-branches", ) +@deprecated_route def origin_branches_browse(request): - """Django view that produces an HTML display of the list of branches + """ + This route is deprecated, use /snapshot-branches/ instead + + Django view that produces an HTML display of the list of branches associated to an origin for a given visit. The URL that points to it is :http:get:`/browse/origin/branches/` """ - return browse_snapshot_branches( - request, - origin_url=request.GET.get("origin_url"), - snapshot_id=request.GET.get("snapshot"), - timestamp=request.GET.get("timestamp"), - branch_name_include=request.GET.get("name_include"), - ) @browse_route( @@ -170,8 +191,12 @@ r"origin/(?P.+)/branches/", view_name="browse-origin-branches-legacy", ) +@deprecated_route def origin_branches_browse_legacy(request, origin_url, timestamp=None): - """Django view that produces an HTML display of the list of branches + """ + This route is deprecated, use /snapshot-branches/ instead + + Django view that produces an HTML display of the list of branches associated to an origin for a given visit. The URLs that point to it are @@ -179,12 +204,6 @@ :http:get:`/browse/origin/(origin_url)/visit/(timestamp)/branches/` """ - return browse_snapshot_branches( - request, - origin_url=origin_url, - snapshot_id=request.GET.get("snapshot"), - timestamp=timestamp, - ) @browse_route( diff --git a/swh/web/browse/views/snapshot.py b/swh/web/browse/views/snapshot.py --- a/swh/web/browse/views/snapshot.py +++ b/swh/web/browse/views/snapshot.py @@ -13,10 +13,19 @@ browse_snapshot_directory, browse_snapshot_log, browse_snapshot_releases, + get_snapshot_context, ) from swh.web.common.utils import reverse +def get_snapshot_from_request(request): + args = {key: value for key, value in request.GET.items()} + snapshot = args.get("snapshot") or args.get("snapshot_id") + if snapshot: + return snapshot + return get_snapshot_context(**args)["snapshot_id"] + + @browse_route( r"snapshot/(?P[0-9a-f]+)/", view_name="browse-snapshot", @@ -115,31 +124,39 @@ @browse_route( r"snapshot/(?P[0-9a-f]+)/log/", + r"snapshot-log/", view_name="browse-snapshot-log", 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 the commit log) collected in a snapshot. The url that points to it is :http:get:`/browse/snapshot/(snapshot_id)/log/` """ + if not snapshot_id: + # This case happens when redirected from /origin/log + snapshot_id = get_snapshot_from_request(request) return browse_snapshot_log(request, snapshot_id=snapshot_id) @browse_route( r"snapshot/(?P[0-9a-f]+)/branches/", + r"snapshot-branches/", view_name="browse-snapshot-branches", checksum_args=["snapshot_id"], ) -def snapshot_branches_browse(request, snapshot_id): +def snapshot_branches_browse(request, snapshot_id=None): """Django view that produces an HTML display of the list of releases collected in a snapshot. The url that points to it is :http:get:`/browse/snapshot/(snapshot_id)/branches/` """ + if not snapshot_id: + # This case happens when redirected from /origin/branches + snapshot_id = get_snapshot_from_request(request) return browse_snapshot_branches(request, snapshot_id=snapshot_id) diff --git a/swh/web/tests/browse/views/test_origin.py b/swh/web/tests/browse/views/test_origin.py --- a/swh/web/tests/browse/views/test_origin.py +++ b/swh/web/tests/browse/views/test_origin.py @@ -861,12 +861,20 @@ ): url = reverse(f"browse-origin-{browse_context}") - resp = check_html_get_response( - client, url, status_code=400, template_used="error.html" - ) - assert_contains( - resp, "An origin URL must be provided as query parameter.", status_code=400 - ) + if browse_context in ["log", "branches"]: + # 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 f"/snapshot-{browse_context}/" in resp.url + else: + resp = check_html_get_response( + client, url, status_code=400, template_used="error.html" + ) + assert_contains( + resp, + "An origin URL must be provided as query parameter.", + status_code=400, + ) def _origin_content_view_test_helper( @@ -1160,16 +1168,19 @@ url = reverse("browse-origin-branches", query_params=query_params) + resp = check_html_get_response(client, url, status_code=302) + redirected_url = resp.url + assert "snapshot-branches" in redirected_url + resp = check_html_get_response( - client, url, status_code=200, template_used="browse/branches.html" + client, redirected_url, status_code=200, template_used="browse/branches.html" ) - origin_branches = origin_snapshot[0] origin_releases = origin_snapshot[1] - origin_branches_url = reverse("browse-origin-branches", query_params=query_params) + origin_branches_url = reverse("browse-snapshot-branches", query_params=query_params) - assert_contains(resp, f'href="{escape(origin_branches_url)}"') + assert_contains(resp, f'"{escape(origin_branches_url)}"') assert_contains(resp, f"Branches ({snapshot_sizes['revision']})") origin_releases_url = reverse("browse-origin-releases", query_params=query_params) @@ -1285,7 +1296,9 @@ ) archive_data.origin_visit_status_add([visit_status]) - url = reverse("browse-origin-branches", query_params={"origin_url": new_origin.url}) + url = reverse( + "browse-snapshot-branches", query_params={"origin_url": new_origin.url} + ) resp = check_html_get_response( client, url, status_code=200, template_used="browse/branches.html" @@ -1310,7 +1323,7 @@ assert_not_contains(resp, "refs/pull/") # check no pull request branches are displayed in the branches view - url = reverse("browse-origin-branches", query_params={"origin_url": origin.url}) + url = reverse("browse-snapshot-branches", query_params={"origin_url": origin.url}) resp = check_html_get_response( client, url, status_code=200, template_used="browse/branches.html" )