Skip to content

feat: add ntfy support#3462

Open
pauln17 wants to merge 18 commits intobluewave-labs:developfrom
pauln17:feat/ntfy-support
Open

feat: add ntfy support#3462
pauln17 wants to merge 18 commits intobluewave-labs:developfrom
pauln17:feat/ntfy-support

Conversation

@pauln17
Copy link
Copy Markdown

@pauln17 pauln17 commented Apr 4, 2026

Describe your changes

Briefly describe the changes you made and their purpose.

Added support for ntfy (notification option), allowing for Checkmate to send alerts (test + real notifications). Handled authentication methods for ntfy (basic username + password or bearer access token) for protected topics.

UI changes (Added section for NTFY):
{38F8893D-C437-4427-9F8C-10E6D3B049BA}

Write your issue number after "Fixes "

Fixes #2817

Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.

  • (Do not skip this or your PR will be closed) I deployed the application locally.
  • (Do not skip this or your PR will be closed) I have performed a self-review and testing of my code.
  • I have included the issue # in the PR.
  • I have added i18n support to visible strings (instead of <div>Add</div>, use):
const { t } = useTranslation();
<div>{t('add')}</div>
  • I have not included any files that are not related to my pull request, including package-lock and package-json if dependencies have not changed
  • I didn't use any hardcoded values (otherwise it will not scale, and will make it difficult to maintain consistency across the application).
  • I made sure font sizes, color choices etc are all referenced from the theme. I don't have any hardcoded dimensions.
  • My PR is granular and targeted to one specific feature.
  • I ran npm run format in server and client directories, which automatically formats your code.
  • I took a screenshot or a video and attached to this PR if there is a UI change.

Additional Notes for Reviewers

  • First contribution here, happy to adjust based on feedback.
  • I am concerned about storing user/pass in DB, not sure if this is correct approach.
  • Ntfy errors still come back as a generic “send failed” style message, so users cannot tell a wrong password or token from a bad URL or network issue. If both username and password and an access token are saved, the server builds Basic auth from the username and password first, so the access token is not used until those fields are cleared.
  • I did run npm run format (client + server) but it seems to update a large number of files (400+); most of that diff is mechanical (whitespace/line endings), so I am not sure if I should commit that.

@akashmannil
Copy link
Copy Markdown
Collaborator

Hey @pauln17, please make sure your Prettier is updated. Even then, if you see widespread changes in the source control panel, once you stage the files, other file changes should be ignored/not added.

Before doing this, could you please confirm whether there are actual backspaces or if it's just showing file changes in the source control panel?

@pauln17
Copy link
Copy Markdown
Author

pauln17 commented Apr 4, 2026

Hi @akashmannil,

my prettier versions are 3.6.2 on server and 3.8.1 on client, I reinstalled a couple times. I ran format on only files I changed this time and it is all visible changes now.

Edit (Two Things):

  • I noticed that the structure mismatch in the locales was fixed, since I added a ntfy section under notifications for en.json I could add it for other languages as well, or do it under a separate PR.
  • PR Feat/webhook auth #3454 implements authType, user and password which is similar to my changes, I can restructure or wait for that PR to get merged and re-implement.

@akashmannil
Copy link
Copy Markdown
Collaborator

Hey @pauln17,

if possible after running npm run format on server/client directories and staging those files, can you please share me the ss of source control panel in VS code(or are you using any other IDE)?

Clarification for 1 - you can raise another PR later for additions.

Clarification 2 - My take is wait for the PR to merge, then rebase and make changes. Im not sure, if we're expecting more changes in that PR

@pauln17
Copy link
Copy Markdown
Author

pauln17 commented Apr 6, 2026

Hi @akashmannil,

Thanks for the clarifications. The most recent commit should show all the changes for running npm run format on only the files I touched, unless I am missing something.

If you specifically want, I could checkout to the previous commit, run format and send you the SS.

I am using VS Code via Cursor.

Copy link
Copy Markdown
Collaborator

@ajhollid ajhollid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!

This PR is currently not thoroughly tested so I won't review it further than I have.

Please thoroughly test the PR and make sure the implementation is technically correct before we get into stylistic issues.

PRs that aren't fully functional cannot be properly reviewed.

Thanks for your interest in the project, we'll be happy to review this more thoroughly when it is in working order!

@pauln17 pauln17 force-pushed the feat/ntfy-support branch from 352383f to b43c886 Compare April 7, 2026 20:54
@pauln17
Copy link
Copy Markdown
Author

pauln17 commented Apr 8, 2026

Hi @ajhollid,

Thanks for your time and advice. I had misunderstood how the database side works, so for now I reverted the Timescale-related changes and implemented ntfy support using the default database, MongoDB.

Most of my implementation and testing was done with MongoDB, and to the best of my knowledge it should be working properly. I’m still actively trying to better understand how Timescale is set up, especially around migrations. If that work belongs in this PR, I’m happy to continue with it, but if it makes more sense as a separate PR, let me know.

I also used PR #3454 as a reference in a few places, including authType, sanitizeNotifications, and some related patterns.

I’m still new to contributing here, so I really appreciate the feedback and I’m doing my best to learn and improve as I go.

@ajhollid
Copy link
Copy Markdown
Collaborator

ajhollid commented Apr 9, 2026

Hi @ajhollid,

Thanks for your time and advice. I had misunderstood how the database side works, so for now I reverted the Timescale-related changes and implemented ntfy support using the default database, MongoDB.

Most of my implementation and testing was done with MongoDB, and to the best of my knowledge it should be working properly. I’m still actively trying to better understand how Timescale is set up, especially around migrations. If that work belongs in this PR, I’m happy to continue with it, but if it makes more sense as a separate PR, let me know.

I also used PR #3454 as a reference in a few places, including authType, sanitizeNotifications, and some related patterns.

I’m still new to contributing here, so I really appreciate the feedback and I’m doing my best to learn and improve as I go.

No problem, we all start off somewhere 👍 Due to the significant investment in resources it takes to properly review a PR we do need to be very selective in what we review.

That said, it is important that any PR that modifies the DB schema does so for both MongoDB and TimescaleDB. What would happen if the change was only made to the MongoDB schemas, but a user is using TimescaleDB? Since this change is at the database level, the application will still compile, but we will have a runtime error when the user tries to use the new feature.

So the answer to your question is yes, the TimescaleDB implementation is required for this feature as you are touching hte database layer.

TimescaleDB is an extension of PostgreSQL so there really isn't anything fancy going on there, especially in the context of notifications. All that is really required here is to add a migration to add the new columns, and update the parameterized queries.

Hope that helps point you in the right direction!

@pauln17
Copy link
Copy Markdown
Author

pauln17 commented Apr 10, 2026

Thanks @ajhollid,

I apologize if mentioning you is incorrect.

I pushed a recent commit for ntfy compatibility with timescale/postgres, but I have a few concerns:

  • To the best of my understanding, migrations have a create function and a drop function, where drop reverts what create does if we need to roll back. I added ntfy to the notification_type enum, but since PostgreSQL does not support removing enum values, I can't revert that addition, so this part is currently one-way. Is that fine?
  • sanitizeNotification prevents username, password, and accessToken from being sent in the response. As a result, when a user goes to configure a notification they added previously, those stripped fields will be empty and only show the placeholder.

@pranav-bhatkar
Copy link
Copy Markdown

pranav-bhatkar commented Apr 10, 2026

hey @pauln17, heads up: i don't know this codebase and i didn't read through the complete code. i just really want ntfy support in checkmate, so i ran this pr end-to-end against my own self-hosted ntfy to see how it behaves. hope it helps.

tested feat/ntfy-support at ac2e190 on mongodb.

blocker: missing migration file

server/src/db/migration/timescaledb/index.ts imports 0022_create_auth_fields on lines 24 and 56, but the file isn't in the pr. npm run dev crashes on boot with ERR_MODULE_NOT_FOUND even on mongodb, because node's esm resolver walks every top-level import before the db selector runs.

i got past it locally with a no-op stub just so i could test the rest. the real file looks like it just needs to follow the 0021_create_retention_compression.ts pattern: two async functions calling pool.query() to add and drop auth_type, username, password, access_token on notifications.

Screenshot 2026-04-10 at 8 23 38 PM

happy paths

after the stub, all three auth modes work against a real ntfy:

auth topic result
none anonymous-writable ✅ delivered
basic acl-protected ✅ delivered
bearer acl-protected ✅ delivered

two bugs, both matching your own notes

form leaks stale fields across auth type changes. switch from bearer to basic and accessToken is still in the post body. authType: "none" sends all three. the server picks the right auth at request time, but a saved notification ends up with extra fields in the db. looks like a reset on the auth-type onChange in client/src/Pages/Notifications/create/index.tsx would do it.

Screenshot_2026-04-10_at_8_25_34 PM

every failure shows "sending notification failed". the status code from got gets dropped in the catch inside ntfy.ts, so 401, 404, and network errors all look the same to the user. capturing err.response?.statusCode and mapping the common codes would already be a big ux win.

happy to re-run the same matrix once the migration file lands.

@pauln17 pauln17 force-pushed the feat/ntfy-support branch from ac2e190 to 2c14982 Compare April 10, 2026 19:15
@pauln17
Copy link
Copy Markdown
Author

pauln17 commented Apr 10, 2026

Hey @pranav-bhatkar, I really appreciate the review and testing you did!

For the missing migration file, it looks like the TimescaleDB migration directory was being gitignored, so I did not realize the file had not been committed. That should be fixed now.

As for the two bugs:

  • The most recent commit should prevent stale data from persisting when authType changes.
  • I can look into this as well, though it may be better handled in a separate PR. I assume the other providers also do not currently map common error codes.

@pauln17 pauln17 force-pushed the feat/ntfy-support branch from 5a529b1 to 74929de Compare April 13, 2026 23:26
@pauln17 pauln17 requested a review from ajhollid April 14, 2026 18:10
@pauln17 pauln17 force-pushed the feat/ntfy-support branch 2 times, most recently from 83e5e28 to 035245b Compare April 16, 2026 22:09
@pauln17 pauln17 force-pushed the feat/ntfy-support branch from 035245b to 89ba500 Compare April 19, 2026 11:02
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.

Add ntfy support to Checkmate

4 participants