From 0171551ecdcd3bcf3e20a3b1f2cdc99584b36333 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 16 Nov 2023 18:14:02 +0100 Subject: [PATCH 1/3] Allow relative action URLs in broadcasts --- client/src/schema/schema.ts | 3 +-- lib/galaxy/schema/notifications.py | 10 +++++++--- lib/galaxy/schema/types.py | 4 ++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts index ea4c0b7fab7b..674134b34a34 100644 --- a/client/src/schema/schema.ts +++ b/client/src/schema/schema.ts @@ -1640,10 +1640,9 @@ export interface components { action_name: string; /** * Link - * Format: uri * @description The link to be opened when the button is clicked. */ - link: string; + link: string | string; }; /** * ArchiveHistoryRequestPayload diff --git a/lib/galaxy/schema/notifications.py b/lib/galaxy/schema/notifications.py index 3a903de737d0..2deb3b5d72a3 100644 --- a/lib/galaxy/schema/notifications.py +++ b/lib/galaxy/schema/notifications.py @@ -8,7 +8,6 @@ ) from pydantic import ( - AnyUrl, Field, Required, ) @@ -22,7 +21,10 @@ EncodedDatabaseIdField, ) from galaxy.schema.schema import Model -from galaxy.schema.types import OffsetNaiveDatetime +from galaxy.schema.types import ( + AbsoluteOrRelativeUrl, + OffsetNaiveDatetime, +) class NotificationVariant(str, Enum): @@ -73,7 +75,9 @@ class ActionLink(Model): action_name: str = Field( Required, title="Action name", description="The name of the action, will be the button title." ) - link: AnyUrl = Field(Required, title="Link", description="The link to be opened when the button is clicked.") + link: AbsoluteOrRelativeUrl = Field( + Required, title="Link", description="The link to be opened when the button is clicked." + ) # Create the corresponding model for the registered category below and diff --git a/lib/galaxy/schema/types.py b/lib/galaxy/schema/types.py index 49b421269077..39d1f45c7959 100644 --- a/lib/galaxy/schema/types.py +++ b/lib/galaxy/schema/types.py @@ -1,5 +1,7 @@ from datetime import datetime +from typing import Union +from pydantic import AnyUrl from pydantic.datetime_parse import parse_datetime from typing_extensions import Literal @@ -7,6 +9,8 @@ # Making them an alias of `str` for now RelativeUrl = str +AbsoluteOrRelativeUrl = Union[AnyUrl, RelativeUrl] + LatestLiteral = Literal["latest"] From 0ce70fb7b937ea02b2518d4b362329fabae58cf2 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 16 Nov 2023 19:01:50 +0100 Subject: [PATCH 2/3] Add test for broadcasts action links --- test/integration/test_notifications.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/integration/test_notifications.py b/test/integration/test_notifications.py index 412fab3e2a82..ed8cc6009f9a 100644 --- a/test/integration/test_notifications.py +++ b/test/integration/test_notifications.py @@ -7,6 +7,7 @@ Dict, List, Optional, + Tuple, ) from uuid import uuid4 @@ -278,6 +279,25 @@ def test_notification_input_dates_consider_timezone(self): assert notification["publication_time"] == "2021-01-01T10:00:00" assert notification["expiration_time"] == "2021-01-01T12:00:00" + def test_broadcast_notification_action_links(self): + # Broadcast notifications can have relative and absolute links + response = self._send_broadcast_notification( + action_links=[("View Workflows", "/workflows/list"), ("Go to GTN", "https://training.galaxyproject.org")] + ) + notification_id = response["notification"]["id"] + response = self._get(f"notifications/broadcast/{notification_id}") + self._assert_status_code_is_ok(response) + notification = response.json() + assert "content" in notification + assert "action_links" in notification["content"] + assert len(notification["content"]["action_links"]) == 2 + action_link = notification["content"]["action_links"][0] + assert action_link["action_name"] == "View Workflows" + assert action_link["link"] == "/workflows/list" + action_link = notification["content"]["action_links"][1] + assert action_link["action_name"] == "Go to GTN" + assert action_link["link"] == "https://training.galaxyproject.org" + def test_sharing_items_creates_notifications_when_expected(self): user1 = self._create_test_user() user2 = self._create_test_user() @@ -359,12 +379,17 @@ def _send_broadcast_notification( message: Optional[str] = None, publication_time: Optional[datetime] = None, expiration_time: Optional[datetime] = None, + action_links: Optional[List[Tuple[str, str]]] = None, ): payload = notification_broadcast_test_data() if subject is not None: payload["content"]["subject"] = subject if message is not None: payload["content"]["message"] = message + if action_links is not None: + payload["content"]["action_links"] = [ + {"action_name": action_name, "link": link} for action_name, link in action_links + ] if publication_time is not None: payload["publication_time"] = publication_time.isoformat() if expiration_time is not None: From 6dbd2333cb11af41d7b05864fb3d7e051ad59931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B3pez?= <46503462+davelopez@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:15:34 +0100 Subject: [PATCH 3/3] Keep AbsoluteOrRelativeUrl as RelativeUrl for now This will always fall back to a string, but the intention of the code reads better and we may add some custom validation later to really check for the URL correctness. Co-authored-by: Marius van den Beek --- client/src/schema/schema.ts | 2 +- lib/galaxy/schema/types.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/client/src/schema/schema.ts b/client/src/schema/schema.ts index 674134b34a34..70261437eb75 100644 --- a/client/src/schema/schema.ts +++ b/client/src/schema/schema.ts @@ -1642,7 +1642,7 @@ export interface components { * Link * @description The link to be opened when the button is clicked. */ - link: string | string; + link: string; }; /** * ArchiveHistoryRequestPayload diff --git a/lib/galaxy/schema/types.py b/lib/galaxy/schema/types.py index 39d1f45c7959..99719cd58100 100644 --- a/lib/galaxy/schema/types.py +++ b/lib/galaxy/schema/types.py @@ -1,7 +1,5 @@ from datetime import datetime -from typing import Union -from pydantic import AnyUrl from pydantic.datetime_parse import parse_datetime from typing_extensions import Literal @@ -9,7 +7,8 @@ # Making them an alias of `str` for now RelativeUrl = str -AbsoluteOrRelativeUrl = Union[AnyUrl, RelativeUrl] +# TODO: we may want to add a custom validator for this and for RelativeUrl +AbsoluteOrRelativeUrl = RelativeUrl LatestLiteral = Literal["latest"]