Merge pull request #559 from GrahamDumpleton/redirection-query-params

Fixup addition of redirection query param for notification.

project-docs/release-notes/version-3.0.0.md

@@ -120,3 +120,10 @@ Bugs Fixed
   had not been created via the REST API but by the web interface would result in
   an internal error. Now properly disallow the request for this case and return
   an error saying session cannot be reacquired.
+
+* When an index URL was supplied to the training portal in the `TrainingPortal`
+  resource, or via the REST API, if the URL had query string parameters, the
+  query string param added by the training platform with the notification
+  message, was not being merged with the existing set of query string parameters
+  and was instead being added to the value of the last query string parameter in
+  the URL.

training-portal/src/project/apps/workshops/views/environment.py

@@ -36,6 +36,7 @@
 from ..manager.sessions import retrieve_session_for_user
 from ..manager.locking import resources_lock
 from ..models import TrainingPortal, Environment, EnvironmentState, SessionState
+from .helpers import update_query_params
 
 
 @login_required
@@ -56,12 +57,22 @@ def environment(request, name):
         instance = Environment.objects.get(name=name)
     except Environment.DoesNotExist:
         if index_url:
-            return redirect(index_url + "?notification=workshop-invalid")
+            return redirect(
+                update_query_params(index_url, {"notification": "workshop-invalid"})
+            )
 
         if not request.user.is_staff and settings.PORTAL_INDEX:
-            return redirect(settings.PORTAL_INDEX + "?notification=workshop-invalid")
+            return redirect(
+                update_query_params(
+                    settings.PORTAL_INDEX, {"notification": "workshop-invalid"}
+                )
+            )
 
-        return redirect(reverse("workshops_catalog") + "?notification=workshop-invalid")
+        return redirect(
+            update_query_params(
+                reverse("workshops_catalog"), {"notification": "workshop-invalid"}
+            )
+        )
 
     # Retrieve a session for the user for this workshop environment.
 
@@ -71,12 +82,22 @@ def environment(request, name):
         return redirect("workshops_session", name=session.name)
 
     if index_url:
-        return redirect(index_url + "?notification=session-unavailable")
+        return redirect(
+            update_query_params(index_url, {"notification": "session-unavailable"})
+        )
 
     if not request.user.is_staff and settings.PORTAL_INDEX:
-        return redirect(settings.PORTAL_INDEX + "?notification=session-unavailable")
+        return redirect(
+            update_query_params(
+                settings.PORTAL_INDEX, {"notification": "session-unavailable"}
+            )
+        )
 
-    return redirect(reverse("workshops_catalog") + "?notification=session-unavailable")
+    return redirect(
+        update_query_params(
+            reverse("workshops_catalog"), {"notification": "session-unavailable"}
+        )
+    )
 
 
 @require_http_methods(["GET"])

training-portal/src/project/apps/workshops/views/helpers.py

@@ -0,0 +1,46 @@
+"""Helper functions for dealing with URLs."""
+
+from urllib.parse import urlparse, parse_qs, urlencode, urlunparse, urljoin
+
+
+def update_query_params(url, params):
+    """Update the query parameters of a URL with the given parameters. If the URL
+    is malformed or cannot be parsed, the original URL is returned."""
+
+    try:
+        # Parse the URL.
+
+        parsed_url = urlparse(url)
+
+        # Handle URLs with no scheme or netloc (e.g., paths like '/page').
+
+        if not parsed_url.scheme and not parsed_url.netloc:
+            # Treat it as a relative path URL.
+
+            base_url = "http://dummy"  # Temporary base for relative path handling
+            parsed_url = urlparse(urljoin(base_url, url))
+
+        # Parse existing query parameters.
+
+        query_params = parse_qs(parsed_url.query)
+
+        # Update or add the new parameters.
+
+        query_params.update({key: [value] for key, value in params.items()})
+
+        # Reconstruct the URL with the updated query string.
+
+        updated_query = urlencode(query_params, doseq=True)
+        updated_url = urlunparse(parsed_url._replace(query=updated_query))
+
+        # If the URL was originally a path, strip out the dummy scheme and netloc.
+
+        if parsed_url.scheme == "http" and parsed_url.netloc == "dummy":
+            return updated_url.replace("http://dummy", "")
+
+        return updated_url
+
+    except Exception:  # pylint: disable=broad-except
+        # In case of any parsing errors, return the original URL.
+
+        return url

training-portal/src/project/apps/workshops/views/session.py

@@ -42,6 +42,7 @@
 from ..manager.sessions import update_session_status, create_request_resources
 from ..manager.analytics import report_analytics_event
 from ..models import TrainingPortal, SessionState
+from .helpers import update_query_params
 
 
 @login_required(login_url="/")
@@ -67,25 +68,35 @@ def session(request, name):
 
     if not instance:
         if index_url:
-            return redirect(index_url + "?notification=session-invalid")
+            return redirect(
+                update_query_params(index_url, {"notification": "session-invalid"})
+            )
 
         if not request.user.is_staff and settings.PORTAL_INDEX:
-            return redirect(settings.PORTAL_INDEX + "?notification=session-invalid")
-
-        return redirect(reverse("workshops_catalog") + "?notification=session-invalid")
+            return redirect(
+                update_query_params(
+                    settings.PORTAL_INDEX, {"notification": "session-invalid"}
+                )
+            )
+
+        return redirect(
+            update_query_params(
+                reverse("workshops_catalog"), {"notification": "session-invalid"}
+            )
+        )
 
     context["session"] = instance
     context["session_owner"] = instance.owner and instance.owner.get_username() or ""
 
-    context[
-        "session_url"
-    ] = f"{settings.INGRESS_PROTOCOL}://{instance.name}.{settings.INGRESS_DOMAIN}"
+    context["session_url"] = (
+        f"{settings.INGRESS_PROTOCOL}://{instance.name}.{settings.INGRESS_DOMAIN}"
+    )
 
     portal_url = f"{settings.INGRESS_PROTOCOL}://{settings.PORTAL_HOSTNAME}"
 
-    context[
-        "restart_url"
-    ] = f"{portal_url}/workshops/session/{instance.name}/delete/?notification=startup-timeout"
+    context["restart_url"] = (
+        f"{portal_url}/workshops/session/{instance.name}/delete/?notification=startup-timeout"
+    )
     context["startup_timeout"] = instance.environment.overdue.total_seconds()
 
     try:
@@ -217,12 +228,22 @@ def session_delete(request, name):
 
     if not instance:
         if index_url:
-            return redirect(index_url + "?notification=session-invalid")
+            return redirect(
+                update_query_params(index_url, {"notification": "session-invalid"})
+            )
 
         if not request.user.is_staff and settings.PORTAL_INDEX:
-            return redirect(settings.PORTAL_INDEX + "?notification=session-invalid")
-
-        return redirect(reverse("workshops_catalog") + "?notification=session-invalid")
+            return redirect(
+                update_query_params(
+                    settings.PORTAL_INDEX, {"notification": "session-invalid"}
+                )
+            )
+
+        return redirect(
+            update_query_params(
+                reverse("workshops_catalog"), {"notification": "session-invalid"}
+            )
+        )
 
     # Mark the instance as stopping now so that it will not be picked up
     # by the user again if they attempt to create a new session immediately.
@@ -239,12 +260,18 @@ def session_delete(request, name):
         notification = "session-deleted"
 
     if index_url:
-        return redirect(index_url + f"?notification={notification}")
+        return redirect(update_query_params(index_url, {"notification": notification}))
 
     if not request.user.is_staff and settings.PORTAL_INDEX:
-        return redirect(settings.PORTAL_INDEX + f"?notification={notification}")
-
-    return redirect(reverse("workshops_catalog") + f"?notification={notification}")
+        return redirect(
+            update_query_params(settings.PORTAL_INDEX, {"notification": notification})
+        )
+
+    return redirect(
+        update_query_params(
+            reverse("workshops_catalog"), {"notification": notification}
+        )
+    )
 
 
 @protected_resource()