Skip to content

feat(webhook): add support for dynamic placeholders in webhook URL#1491

Merged
fallenbagel merged 6 commits into
seerr-team:developfrom
0xSysR3ll:feature-webhook-placeholders
Sep 10, 2025
Merged

feat(webhook): add support for dynamic placeholders in webhook URL#1491
fallenbagel merged 6 commits into
seerr-team:developfrom
0xSysR3ll:feature-webhook-placeholders

Conversation

@0xSysR3ll
Copy link
Copy Markdown
Contributor

@0xSysR3ll 0xSysR3ll commented Mar 17, 2025

Description

This PR introduces support for dynamic placeholders in webhook URLs, starting with {{requestedBy_username}}.
Users can enable this feature in the notification settings, and available placeholders are displayed as a list.

  • Should we rename "placeholders" to "variables" or "tokens" for better clarity?
  • Is the UI placement of the placeholders list intuitive, or should it be adjusted?
  • Should we extend support for additional placeholders in future updates?

N.B: Marked this feature as experimental to gather feedback before finalization.

Screenshot (if UI-related)

image

To-Dos

  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

Issues Fixed or Closed

@0xSysR3ll 0xSysR3ll marked this pull request as draft March 17, 2025 23:03
@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from e66b767 to 508fd82 Compare March 26, 2025 16:10
@0xSysR3ll 0xSysR3ll marked this pull request as ready for review March 26, 2025 16:18
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2025

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@github-actions github-actions Bot added the merge conflict Cannot merge due to merge conflicts label Apr 8, 2025
@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from 51bd6ea to 2863156 Compare April 10, 2025 17:48
@github-actions github-actions Bot removed the merge conflict Cannot merge due to merge conflicts label Apr 10, 2025
Comment thread server/lib/notifications/agents/webhook.ts Fixed
@github-actions github-actions Bot added the merge conflict Cannot merge due to merge conflicts label May 9, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2025

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from 08e2338 to acfd98a Compare May 9, 2025 12:28
@github-actions github-actions Bot removed the merge conflict Cannot merge due to merge conflicts label May 9, 2025
@gauthier-th
Copy link
Copy Markdown
Member

Should we rename "placeholders" to "variables" or "tokens" for better clarity?

IMO "variables" is more clear, but "placeholders" is fine too.

Is the UI placement of the placeholders list intuitive, or should it be adjusted? (Open to feedback!)

It may quickly be a bit messy if the list is too long, don't you think so?
We could just add a link to the documentation, like here: https://docs.jellyseerr.dev/using-jellyseerr/notifications/webhook#template-variables ?

Should we extend support for additional placeholders in future updates?

What's wrong with using the same placeholder system as current one in the webhook content itself?

@fallenbagel fallenbagel changed the title feat(wehbook): add support for dynamic placeholders in webhook URL feat(webhook): add support for dynamic placeholders in webhook URL Aug 29, 2025
@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from acfd98a to 5ed97de Compare August 31, 2025 19:25
@0xSysR3ll
Copy link
Copy Markdown
Contributor Author

IMO "variables" is more clear, but "placeholders" is fine too.

Yep, totally makes sense !

It may quickly be a bit messy if the list is too long, don't you think so? We could just add a link to the documentation, like here: https://docs.jellyseerr.dev/using-jellyseerr/notifications/webhook#template-variables ?

As there was only one variable available, it wasn't necessary. But since we include them all, then yes I will do this.

What's wrong with using the same placeholder system as current one in the webhook content itself?

I did not think about making all variables available at first, but I guess you're right, it could be interesting for some users.

Comment thread src/components/Settings/Notifications/NotificationsWebhook/index.tsx Outdated
@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from 54d7c6a to ce2c695 Compare September 3, 2025 20:11
Comment thread src/components/Settings/Notifications/NotificationsWebhook/index.tsx Outdated
0xSysR3ll and others added 6 commits September 9, 2025 22:17
… update related logic

Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
Co-authored-by: Gauthier <mail@gauthierth.fr>
Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
@0xSysR3ll 0xSysR3ll force-pushed the feature-webhook-placeholders branch from c95d69a to 0cb28d9 Compare September 9, 2025 20:18
@fallenbagel fallenbagel merged commit 17172e9 into seerr-team:develop Sep 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for dynamic notification URLs

4 participants