Nostr dm notifications #3051

zappityzap · 2023-04-11T02:53:41Z

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

I have read and understand the pull request rules.

Description

Adds notification provider for the nostr protocol: https://github.com/nostr-protocol/nostr

Notifications are sent as encrypted direct messages.

Type of change

New feature (non-breaking change which adds functionality)

Checklist

My code follows the style guidelines of this project
I ran ESLint and other linters for modified files
I have performed a self-review of my own code and tested it
I have commented my code, particularly in hard-to-understand areas
(including JSDoc for methods)
My changes generate no new warnings
My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

CommanderStorm

Overall this seems pretty dinished, despite being a draft.
Is this intentional?

I left some code style marks

server/notification-providers/nostr.js

CommanderStorm · 2023-05-30T14:13:25Z

server/notification-providers/nostr.js

@@ -0,0 +1,108 @@
+const NotificationProvider = require("./notification-provider");
+global.crypto = require("crypto");


Why is this line included?

Suggested change

global.crypto = require("crypto");

Without it, on Node 18 there is an error from the nostr-tools module that crypto is not defined. Node 19+ doesn't need it. There is a closed issue for nostr-tools about it: nbd-wtf/nostr-tools#192

I see they have a better method for Node 18 compatibility now, but I'm not sure how to implement it:

// node.js 18 and earlier requires globalThis.crypto polyfill. import { webcrypto } from 'node:crypto'; // @ts-ignore if (!globalThis.crypto) globalThis.crypto = webcrypto;

Here is the error with Node 18:

ReferenceError: crypto is not defined at Object.encrypt (/home/ansible/uptime-kuma/node_modules/nostr-tools/lib/nostr.cjs.js:994:19) at Nostr.send (/home/ansible/uptime-kuma/server/notification-providers/nostr.js:34:44) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async Socket.<anonymous> (/home/ansible/uptime-kuma/server/server.js:1209:27)

Please include a comment that this is a polyfill for node <= 18

CommanderStorm · 2023-05-30T14:17:45Z

server/notification-providers/nostr.js

+    nip04,
+    nip19
+} = require("nostr-tools");
+require("websocket-polyfill");


Why is this included?
No other of the providers include this.

If you think this is needed, let's split this out of this PR.

Without it, on Node 18 there is an error from the nostr-tools module. Node 19+ doesn't need it.

TypeError: Cannot read properties of undefined (reading 'readyState') at Object.close (/home/ansible/uptime-kuma/node_modules/nostr-tools/lib/nostr.cjs.js:566:14) at Nostr.send (/home/ansible/uptime-kuma/server/notification-providers/nostr.js:66:23) at async Socket.<anonymous> (/home/ansible/uptime-kuma/server/server.js:1209:27)

Please include a comment that this is a polyfill for node <= 18

CommanderStorm

Overall this seems pretty dinished, despite being a draft.
Is this intentional?

I left some code style marks

Co-authored-by: Frank Elsinga <frank@elsinga.de>

zappityzap · 2023-05-31T04:11:09Z

Overall this seems pretty dinished, despite being a draft. Is this intentional?

I left some code style marks

Thank you for the review and feedback. I'm new to open source contributions and JavaScript. I wanted to learn more about the Nostr protocol so I implemented this for my own use. The project contribution guidelines said to mark it as a draft.

CommanderStorm · 2023-05-31T12:10:06Z

The project contribution guidelines said to mark it as a draft.

See https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma
Notification Providers are handled differently from general feautre/code changes.
Empty draft PRs are only for discussing significant changes (as they carry significant reviewer effort+may risk breaking things) like #128/#2129/#2693

server/notification-providers/nostr.js

src/components/notifications/Nostr.vue

Co-authored-by: Frank Elsinga <frank@elsinga.de>

# Conflicts: # src/lang/en.json

This reverts commit b8bcb6f.

louislam · 2023-06-28T06:43:29Z

I just realized that this pr is not working on arm v7, because there is no pre-built binary for the bufferutil dependency. So I reverted it.

Feel free to continue this by opening a new pr if you have found a solution.

https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Easy to install for non-Docker users, no native build dependency is needed (for x86_64/armv7/arm64), no extra config, no extra effort required to get it running

* Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace * move closer to where it is used * simplify success or failure logic * don't clobber the non-alert msg * Update server/notification-providers/nostr.js * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <frank@elsinga.de> Co-authored-by: zappityzap <128872140+zappityzap@users.noreply.github.com>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

* Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace * move closer to where it is used * simplify success or failure logic * don't clobber the non-alert msg * Update server/notification-providers/nostr.js * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <frank@elsinga.de> Co-authored-by: zappityzap <128872140+zappityzap@users.noreply.github.com>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

* Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace * move closer to where it is used * simplify success or failure logic * don't clobber the non-alert msg * Update server/notification-providers/nostr.js * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <frank@elsinga.de> Co-authored-by: zappityzap <128872140+zappityzap@users.noreply.github.com>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

* ✨ feat: json-query monitor added Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: import warning error Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: br tag and remove comment Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: supporting compare string with other types Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: switch to a better lib for json query Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: better description on json query and using `v-html` in jsonQueryDescription element to fix `a` tags Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: result variable in error message Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: typos in json query description Co-authored-by: Frank Elsinga <frank@elsinga.de> * 📝 docs: `HTTP(s) Json Query` added to monitor list in `README.md` Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: needed white space in `README.md` Co-authored-by: Frank Elsinga <frank@elsinga.de> * Nostr dm notifications (#3051) * Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace Co-authored-by: Frank Elsinga <frank@elsinga.de> * move closer to where it is used * simplify success or failure logic * don't clobber the non-alert msg * Update server/notification-providers/nostr.js Co-authored-by: Frank Elsinga <frank@elsinga.de> * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <frank@elsinga.de> * Drop nostr * Rebuild package-lock.json * Lint --------- Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> Co-authored-by: Frank Elsinga <frank@elsinga.de> Co-authored-by: zappityzap <128872140+zappityzap@users.noreply.github.com> Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

* ✨ feat: added kafka producer Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: eslint warn Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: typings and auth problems Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: better variable name to trrack disconnection Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: grouping Kafka Producer special settings into one template Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * ✨ feat: add kafka producer translations into `en.json` Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: disable close-on-select on kafka broker picker Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * 🐛 fix: `en.json` invalid json (conflict resolve) Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> * Nostr dm notifications (#3051) * Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace Co-authored-by: Frank Elsinga <frank@elsinga.de> * move closer to where it is used * simplify success or failure logic * don't clobber the non-alert msg * Update server/notification-providers/nostr.js Co-authored-by: Frank Elsinga <frank@elsinga.de> * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <frank@elsinga.de> * Drop nostr * Minor * Fix a bug of clone --------- Signed-off-by: Muhammed Hussein Karimi <info@karimi.dev> Co-authored-by: Frank Elsinga <frank@elsinga.de> Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

This reverts commit b8bcb6f.

zappityzap and others added 5 commits April 6, 2023 08:00

Add nostr DM notification provider

d070a78

require crypto for node 18 compatibility

cd2d7a7

Merge branch 'louislam:master' into nostr-dm-notifications

697e2df

Merge branch 'louislam:master' into nostr-dm-notifications

b3045a6

Merge branch 'louislam:master' into nostr-dm-notifications

bda0f06

CommanderStorm requested changes May 30, 2023

View reviewed changes

zappityzap and others added 4 commits May 30, 2023 19:59

remove whitespace

042d774

Co-authored-by: Frank Elsinga <frank@elsinga.de>

move closer to where it is used

c2140a6

simplify success or failure logic

b48b47b

don't clobber the non-alert msg

94c7049

CommanderStorm reviewed May 31, 2023

View reviewed changes

server/notification-providers/nostr.js Outdated Show resolved Hide resolved

CommanderStorm reviewed May 31, 2023

View reviewed changes

src/components/notifications/Nostr.vue Outdated Show resolved Hide resolved

zappityzap and others added 5 commits May 31, 2023 09:55

Update server/notification-providers/nostr.js

81394ee

Co-authored-by: Frank Elsinga <frank@elsinga.de>

polyfills required for node <= 18

9040863

resolve linter warnings

f85969a

Merge branch 'master' into nostr-dm-notifications

23be2d7

missing comma

e66c4b4

zappityzap marked this pull request as ready for review May 31, 2023 17:50

louislam added this to the 1.23.0 milestone Jun 22, 2023

Merge remote-tracking branch 'origin/master' into nostr-dm-notifications

dbeeae1

# Conflicts: # src/lang/en.json

louislam merged commit b8bcb6f into louislam:master Jun 27, 2023
2 checks passed

louislam added a commit that referenced this pull request Jun 28, 2023

Revert "Nostr dm notifications (#3051)"

a8d0a64

This reverts commit b8bcb6f.

louislam mentioned this pull request Jun 28, 2023

Revert "Nostr dm notifications" #3325

Merged

louislam added a commit that referenced this pull request Jun 28, 2023

Revert "Nostr dm notifications (#3051)" (#3325)

f08e1a4

This reverts commit b8bcb6f.

UptimeKumaBot pushed a commit to UptimeKumaBot/uptime-kuma that referenced this pull request Jun 29, 2023

Revert "Nostr dm notifications (louislam#3051)" (louislam#3325)

7031010

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

UptimeKumaBot pushed a commit to UptimeKumaBot/uptime-kuma that referenced this pull request Jun 30, 2023

Revert "Nostr dm notifications (louislam#3051)" (louislam#3325)

b81d860

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

UptimeKumaBot pushed a commit to UptimeKumaBot/uptime-kuma that referenced this pull request Jul 4, 2023

Revert "Nostr dm notifications (louislam#3051)" (louislam#3325)

19b3958

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <louislam@users.noreply.github.com>

zappityzap mentioned this pull request Jul 23, 2023

Nostr dm notifications #3473

Merged

7 tasks

mhkarimi1383 added a commit to mhkarimi1383/uptime-kuma that referenced this pull request Apr 29, 2024

Revert "Nostr dm notifications (louislam#3051)"

8bfee23

This reverts commit b8bcb6f.

mhkarimi1383 added a commit to mhkarimi1383/uptime-kuma that referenced this pull request Apr 29, 2024

Revert "Nostr dm notifications (louislam#3051)"

6418665

This reverts commit b8bcb6f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nostr dm notifications #3051

Nostr dm notifications #3051

zappityzap commented Apr 11, 2023

CommanderStorm left a comment

CommanderStorm May 30, 2023

zappityzap May 31, 2023

CommanderStorm May 31, 2023

CommanderStorm May 30, 2023

zappityzap May 31, 2023

CommanderStorm May 31, 2023

CommanderStorm left a comment

zappityzap commented May 31, 2023

CommanderStorm commented May 31, 2023

louislam commented Jun 28, 2023

		@@ -0,0 +1,108 @@
		const NotificationProvider = require("./notification-provider");
		global.crypto = require("crypto");

Nostr dm notifications #3051

Nostr dm notifications #3051

Conversation

zappityzap commented Apr 11, 2023

Description

Type of change

Checklist

Screenshots (if any)

CommanderStorm left a comment

Choose a reason for hiding this comment

CommanderStorm May 30, 2023

Choose a reason for hiding this comment

zappityzap May 31, 2023

Choose a reason for hiding this comment

CommanderStorm May 31, 2023

Choose a reason for hiding this comment

CommanderStorm May 30, 2023

Choose a reason for hiding this comment

zappityzap May 31, 2023

Choose a reason for hiding this comment

CommanderStorm May 31, 2023

Choose a reason for hiding this comment

CommanderStorm left a comment

Choose a reason for hiding this comment

zappityzap commented May 31, 2023

CommanderStorm commented May 31, 2023

louislam commented Jun 28, 2023