Skip to content

feat(socialPoster): add Nostr pubkey tagging with hideNostr check in … #2100

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brymut
Copy link
Contributor

@brymut brymut commented Apr 13, 2025

Description

Add Nostr pubkey tagging with hideNostr check in social poster, using the NIP as guidance I've included the item author as a 'p' tag in the npub.

I created a test social post using the socialPoster worker: to check that the tag is added.

closes #2090

Screenshots

Additional Context

Was anything unclear during your work on this PR? Anything we should definitely take a closer look at?

Checklist

Are your changes backwards compatible? Please answer below:
yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
10

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a

Did you introduce any new environment variables? If so, call them out explicitly here:
no

@brymut brymut marked this pull request as ready for review April 14, 2025 16:36
@brymut brymut requested a review from ekzyis April 14, 2025 16:37
@brymut
Copy link
Contributor Author

brymut commented Apr 14, 2025

Hi @ekzyis / @huumn, I believe this is ready for you to have a look.

Comment on lines 53 to 57
await nostr.publish({
created_at: Math.floor(new Date().getTime() / 1000),
content: message,
tags: [],
tags: nostrPubkey ? [['p', nostrPubkey]] : [],
kind: 1
}, {
Copy link
Member

@ekzyis ekzyis Apr 14, 2025

Choose a reason for hiding this comment

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

You are tagging the user, but we want to mention them like in this note:

2025-04-15-004720_628x345_scrot

edit: Sorry, just realized my ticket was not clear on that since I used "tag" in the title but then used "mention" in the description. I meant "mention" like I showed in the example note.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brymut brymut force-pushed the feat/include-nostr-tag branch 2 times, most recently from 14769be to 5fda156 Compare April 15, 2025 19:34
@brymut
Copy link
Contributor Author

brymut commented Apr 15, 2025

Hi @ekzyis , I've addressed all feedback provided, please have another look.

@brymut brymut requested a review from ekzyis April 15, 2025 19:38
@brymut brymut force-pushed the feat/include-nostr-tag branch from 5fda156 to 06d02cd Compare April 15, 2025 19:40

const message = await itemToMessage({ item, postAuthorNostrProfile })
console.log('Message:', message)
await postToTwitter({ message })
Copy link
Member

Choose a reason for hiding this comment

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

This includes the nostr mention in the twitter message. This will look bad on twitter.

return `${item.title}

by ${item.userName} in ~${item.subName}
by ${item.userName}${postAuthorNostrProfile ? `, nostr profile: nostr:${postAuthorNostrProfile}` : ''} in ~${item.subName}
Copy link
Member

Choose a reason for hiding this comment

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

the mention should replace item.userName, not be appended:

2025-04-15-214812_634x259_scrot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was not clarified @ekzyis, I assumed we'd want to know the username on stacker too

@brymut brymut force-pushed the feat/include-nostr-tag branch from 06d02cd to 392e62a Compare April 15, 2025 20:10
@brymut
Copy link
Contributor Author

brymut commented Apr 15, 2025

Please have another look. @ekzyis

@brymut brymut requested a review from ekzyis April 15, 2025 20:12
Copy link
Member

@huumn huumn left a comment

Choose a reason for hiding this comment

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

According to nip 27 you'll also want to add a p tag.


const postAuthor = await models.user.findUnique({
where: { id: item.userId, hideNostr: false },
select: { nostrPubkey: true }
Copy link
Member

Choose a reason for hiding this comment

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

You're going to want to also check nostrAuthPubkey ... sorry, we have two ways of adding a nostr pubkey.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, tested with an account with both assigned as well as either one assigned. Latest seems to work for both.

lib/nostr.js Outdated

export function getNostrProfile (pubkey) {
try {
const { data } = nip19.decode(pubkey)
Copy link
Member

Choose a reason for hiding this comment

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

We store all nostr pubkeys in hex so you don't need to do this + throws an error when it encounters hex.

Copy link
Contributor Author

@brymut brymut Apr 16, 2025

Choose a reason for hiding this comment

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

It errors out requiring when we don't decode first. @huumn
Screenshot 2025-04-16 at 04 34 17

edit: I get what I did wrong here, I added the npub directly into the database instead of through the UI so I could test with a user. I can see how the hex comes in now. Thanks @huumn

@brymut
Copy link
Contributor Author

brymut commented Apr 16, 2025

According to nip 27 you'll also want to add a p tag.

@huumn the p tag is added automatically, please check the event json in the test post here

@brymut brymut force-pushed the feat/include-nostr-tag branch from 3b5ef22 to 760cbbb Compare April 16, 2025 17:52
@brymut
Copy link
Contributor Author

brymut commented Apr 16, 2025

Requested changes addressed, please have a look. @huumn

@brymut brymut requested a review from huumn April 16, 2025 17:54
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.

Tag npub of stacker in social poster
3 participants