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 <[email protected]>

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 <[email protected]>

# 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 <[email protected]> Co-authored-by: zappityzap <[email protected]>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <[email protected]>

* 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 <[email protected]> Co-authored-by: zappityzap <[email protected]>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <[email protected]>

* 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 <[email protected]> Co-authored-by: zappityzap <[email protected]>

This reverts commit b8bcb6f. Co-authored-by: Louis Lam <[email protected]>

* ✨ feat: json-query monitor added Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: import warning error Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: br tag and remove comment Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: supporting compare string with other types Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: switch to a better lib for json query Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: better description on json query and using `v-html` in jsonQueryDescription element to fix `a` tags Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: result variable in error message Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: typos in json query description Co-authored-by: Frank Elsinga <[email protected]> * 📝 docs: `HTTP(s) Json Query` added to monitor list in `README.md` Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: needed white space in `README.md` Co-authored-by: Frank Elsinga <[email protected]> * Nostr dm notifications (#3051) * Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace Co-authored-by: Frank Elsinga <[email protected]> * 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 <[email protected]> * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <[email protected]> * Drop nostr * Rebuild package-lock.json * Lint --------- Signed-off-by: Muhammed Hussein Karimi <[email protected]> Co-authored-by: Frank Elsinga <[email protected]> Co-authored-by: zappityzap <[email protected]> Co-authored-by: Louis Lam <[email protected]>

* ✨ feat: added kafka producer Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: eslint warn Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: typings and auth problems Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: better variable name to trrack disconnection Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: grouping Kafka Producer special settings into one template Signed-off-by: Muhammed Hussein Karimi <[email protected]> * ✨ feat: add kafka producer translations into `en.json` Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: disable close-on-select on kafka broker picker Signed-off-by: Muhammed Hussein Karimi <[email protected]> * 🐛 fix: `en.json` invalid json (conflict resolve) Signed-off-by: Muhammed Hussein Karimi <[email protected]> * Nostr dm notifications (#3051) * Add nostr DM notification provider * require crypto for node 18 compatibility * remove whitespace Co-authored-by: Frank Elsinga <[email protected]> * 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 <[email protected]> * polyfills required for node <= 18 * resolve linter warnings * missing comma --------- Co-authored-by: Frank Elsinga <[email protected]> * Drop nostr * Minor * Fix a bug of clone --------- Signed-off-by: Muhammed Hussein Karimi <[email protected]> Co-authored-by: Frank Elsinga <[email protected]> Co-authored-by: Louis Lam <[email protected]>

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 <[email protected]>

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 <[email protected]>

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

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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