Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow relative URLs in broadcasts action links #17042

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

davelopez
Copy link
Contributor

Fixes #17033

It relaxes the schema typing in notifications a bit to support relative URLs. The pydantic type AnyUrl always confuses me and makes me think it supports both absolute and relative URLs but it's not the case.

How to test the changes?

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-actions github-actions bot added this to the 23.2 milestone Nov 16, 2023
@davelopez davelopez force-pushed the fix_broadcasts_action_url_type branch from 921176a to 81a6006 Compare November 16, 2023 18:06
@davelopez davelopez force-pushed the fix_broadcasts_action_url_type branch from 81a6006 to 5488abb Compare November 17, 2023 09:59
@davelopez davelopez changed the title Allow relative action URLs in broadcasts Allow relative URLs in broadcasts action links Nov 17, 2023
lib/galaxy/schema/types.py Outdated Show resolved Hide resolved
@davelopez davelopez force-pushed the fix_broadcasts_action_url_type branch 2 times, most recently from 8de42b7 to 48cb220 Compare November 21, 2023 09:18
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 <m.vandenbeek@gmail.com>
@davelopez davelopez force-pushed the fix_broadcasts_action_url_type branch from 48cb220 to fe7ad2e Compare November 21, 2023 09:20
@mvdbeek mvdbeek merged commit 0e1324c into galaxyproject:dev Nov 28, 2023
51 checks passed
@mvdbeek
Copy link
Member

mvdbeek commented Nov 28, 2023

Do we need to backport this to 23.1 ?

@davelopez
Copy link
Contributor Author

Yes it might be a good idea, there is no admin panel for it in 23.1 but can be an issue when using the API, I will take care of it 👍

@davelopez davelopez deleted the fix_broadcasts_action_url_type branch November 28, 2023 17:41
@mvdbeek mvdbeek modified the milestones: 23.2, 24.0 Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broadcast Notifications do not Accept Local URLs
2 participants