-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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-service,dashboard): New subscribers page and api #7525
Conversation
307737d
to
8eceeae
Compare
@novu/client
@novu/headless
@novu/node
@novu/notification-center
novu
@novu/providers
@novu/shared
commit: |
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
8eceeae
to
abd81d2
Compare
abd81d2
to
228b1d3
Compare
228b1d3
to
a6f5e01
Compare
a6f5e01
to
1f79320
Compare
after: command.after || undefined, | ||
before: command.before || undefined, | ||
paginateField: 'id', | ||
limit: command.limit, | ||
sortDirection: command.orderDirection, | ||
sortBy: command.orderBy, | ||
query: { | ||
_environmentId: command.user.environmentId, | ||
_organizationId: command.user.organizationId, | ||
$and: [ | ||
{ | ||
...(command.email && { email: { $regex: command.email, $options: 'i' } }), | ||
...(command.phone && { phone: { $regex: command.phone, $options: 'i' } }), | ||
...(command.subscriberId && { subscriberId: command.subscriberId }), | ||
...(command.name && { | ||
$expr: { | ||
$regexMatch: { | ||
input: { $concat: ['$firstName', ' ', '$lastName'] }, | ||
regex: command.name, | ||
options: 'i', | ||
}, | ||
}, | ||
}), | ||
}, | ||
], | ||
}, |
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.
Wouldn't it be better to keep this repo syntax inside the repo, making the exposed methods more reusable, this is super robust in the current implementation
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.
We could add a filters
option down the road, yes.
const params = new URLSearchParams({ | ||
limit: limit.toString(), | ||
...(after && { after }), | ||
...(before && { before }), | ||
...(orderDirection && { orderDirection }), | ||
...(email && { email }), | ||
...(phone && { phone }), | ||
...(subscriberId && { subscriberId }), | ||
...(name && { name }), | ||
...(orderBy && { orderBy }), | ||
...(orderDirection && { orderDirection }), | ||
}); | ||
|
||
const { data } = await getV2<{ data: IListSubscribersResponseDto }>(`/subscribers?${params}`, { | ||
environment, | ||
}); | ||
return data; |
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.
shoudn't we attempt to use the SDK here once I deploy it based on your new work
6aa7f2e
to
b82058f
Compare
ebf7c60
to
01ab130
Compare
01ab130
to
9d7997b
Compare
@IsEnum(DirectionEnum) | ||
@IsOptional() | ||
orderDirection: DirectionEnum = DirectionEnum.DESC; | ||
|
||
@IsEnum(['updatedAt', 'createdAt']) | ||
@IsOptional() | ||
orderBy: 'updatedAt' | 'createdAt' = 'createdAt'; |
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.
by the way, if those optional params have default we can remove the static default values from
apps/api/src/app/subscribers-v2/subscriber.controller.ts
@IsEnum(DirectionEnum) | |
@IsOptional() | |
orderDirection: DirectionEnum = DirectionEnum.DESC; | |
@IsEnum(['updatedAt', 'createdAt']) | |
@IsOptional() | |
orderBy: 'updatedAt' | 'createdAt' = 'createdAt'; | |
@IsEnum(DirectionEnum) | |
@IsOptional() | |
orderDirection?: DirectionEnum = DirectionEnum.DESC; | |
@IsEnum(['updatedAt', 'createdAt']) | |
@IsOptional() | |
orderBy?: 'updatedAt' | 'createdAt' = 'createdAt'; |
let builder = this.MongooseModel.find(paginationQuery) | ||
.sort({ [sortBy]: sortValue, [paginateField]: sortValue }) | ||
.limit(limit + 1); | ||
|
||
if (enhanceQuery) { | ||
builder = enhanceQuery(builder); | ||
} | ||
|
||
const rawResults = await builder.exec(); |
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.
what is enhanceQuery
?
now sure i understand the flow here, won't line 383 will be executed at the time as Promise?
3df7edb
to
f408496
Compare
f408496
to
7b7a16e
Compare
7b7a16e
to
3106a4b
Compare
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer