-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: tags in subscriptions #56
Conversation
Add README |
internal/nostr/models.go
Outdated
Tags nostr.Tags `json:"tags,omitempty"` | ||
Since *nostr.Timestamp `json:"since,omitempty"` | ||
Until *nostr.Timestamp `json:"until,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use the nostr lib types there or our own types that we then pack into a nostr Filter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ig it's best to use our own to keep it consistent 👍
It adds more code not only for the types (for example nostr.Timestamp has methods too) but also requires type conversion for Tags each time we use it in the library methods:
cannot use *subscription.Tags (variable of type TagMap) as "github.com/nbd-wtf/go-nostr".TagMap value in assignment
Best to use nostr lib types for now ig
@im-adithya you have this "ADD README" comment. Can you update the filters in the readme or maybe even a full example with all the fields to be clear or add some examples? is this right for example? (Can you also update the guide?)
|
update the readme as mentioned above and we merge this. |
Done, should be ready to merge! |
so nothing changed? |
@im-adithya how does this work now? did you even test it? |
Lol yes, nothing has to be changed. Just the way tags are supplied is changed (hence those big README changes). The easyjson internally handles passing the tags like this:
Checked everything and works fine! The tags get stored properly in the subscriptions table too. |
Previously we didn't support tags (mistakenly), this fix makes sure tags can be passed alongside the other filter attributes like this