Skip to content
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

feat(api): notifications feed filtering by partial payload object #3939

Merged

Conversation

LetItRock
Copy link
Contributor

What change does this PR introduce?

Allow filtering of the notifications feed by the custom data from the payload object.

The two endpoints GET /subscribers/:subscriberId/notifications/feed and GET /widgets/notifications/feed now can take the query param like ?payload=eyJmb28iOjEyM30=.

The value is a base64 encoded string of the partial payload JSON object.

For example, if the payload passed to the trigger event was like { foo: 123, bar: 'bar' }, then you filter notifications by { foo: 123 } by doing btoa(JSON.stringify({ foo: 123 })) that results in base64 encoded string eyJmb28iOjEyM30= and passing it to the query param.

Why was this change needed?

Allows building custom notification center experiences.

Other information (Screenshots)

Screenshot 2023-08-08 at 18 33 01 Screenshot 2023-08-08 at 18 33 11

@LetItRock LetItRock self-assigned this Aug 8, 2023
@linear
Copy link

linear bot commented Aug 8, 2023

NV-2374 Allow notifications feed filtering by custom payload data

What?

Allow filtering the notifications feed by a value from the custom payload object passed to the trigger event.

Note:

  • we would like to release it with a patch 0.17.2

  • the GitStart has done some work, but we feel that it's not good enough

Why? (Context)

Currently it's only possible to query the notifications feed by: feedIdentifier, read, seen fields. It's a limitation that blocks clients from building custom notification center experiences.

Definition of Done

  • expand the /widgets/notifications/feed and /subscribers/:subscriberId/notifications/feed endpoints to take the query params that will allow to filter by the custom "key" and "value" from the trigger payload
  • example how it's implemented by Strapi: https://strapi.io/blog/deep-filtering-alpha-26
  • implement the e2e tests

Comment on lines +30 to +36
@ApiPropertyOptional({
required: false,
type: 'string',
description: 'Base64 encoded string of the partial payload JSON object',
example: 'btoa(JSON.stringify({ foo: 123 })) results in base64 encoded string like eyJmb28iOjEyM30=',
})
payload?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two feed endpoints now do take the payload query param which is the base64 encoded string of the partial payload JSON object. Check the description for more info.

Comment on lines +24 to +34
private getPayloadObject(payload?: string): object | undefined {
if (!payload) {
return;
}

try {
return JSON.parse(Buffer.from(payload, 'base64').toString());
} catch (e) {
throw new BadRequestException('Invalid payload, the JSON object should be encoded to base64 string.');
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

convert the base64 string payload to the JS object

@@ -47,7 +61,7 @@ export class GetNotificationsFeed {
command.environmentId,
subscriber._id,
ChannelTypeEnum.IN_APP,
{ feedId: command.feedId, seen: command.query.seen, read: command.query.read },
{ feedId: command.feedId, seen: command.query.seen, read: command.query.read, payload },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pass the payload to the repository

Comment on lines +74 to +79
if (query.payload) {
requestQuery = {
...requestQuery,
...getFlatObject({ payload: query.payload }),
};
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the payload object is passed, then prepare the flat object out of it, like if the payload is { foo: { bar: 123 } }, then the flat object would be:

{
"payload.foo.bar": 123
}

Which is passed then to the find query.

Copy link
Contributor

@ainouzgali ainouzgali left a comment

Choose a reason for hiding this comment

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

@LetItRock really awesome work !
As I mentioned before, I feel it will have a lot of impact to add it to the node sdk and headless.
Also, probably not now, but it will be nice to add a function that does the encoding for the user, at least for the node sdk

if (query.payload) {
requestQuery = {
...requestQuery,
...getFlatObject({ payload: query.payload }),
Copy link
Contributor

Choose a reason for hiding this comment

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

what if query.payload is an empty object, { }. Are we still covered with errors/will not throw errors? Also in the conversion of the payload in other places ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the query.payload is {} then the getFlatObject({ payload: {} }) will return an empty object {} ;)
So basically it won't return any keys flattened if the value is an empty object, including nested objects.

@LetItRock LetItRock added this pull request to the merge queue Aug 11, 2023
Merged via the queue into next with commit d0ca3b0 Aug 11, 2023
30 checks passed
@LetItRock LetItRock deleted the nv-2374-notifications-feed-filtering-by-partial-payload branch August 11, 2023 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High priority Created by Linear-GitHub Sync @novu/api @novu/dal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants