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: add endpoint to get novu limits #2495

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

davidsoderberg
Copy link
Contributor

No description provided.

@@ -164,4 +167,24 @@ export class IntegrationsController {
})
);
}

@Get('/:channelType/limit')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Get('/:channelType/limit')
@Get('/:channelType/limits')

As I am not very sure what a limit is, just a briefly reminder if we plan Limit to be a sub-resource with properties to pluralise it.

Copy link
Contributor

Choose a reason for hiding this comment

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

After going through all the PR seems it is meant to be a calculated property. So all good. 👌🏻

export class CalculateLimitNovuIntegration {
constructor(private messageRepository: MessageRepository) {}

static MAX_NOVU_INTEGRATION_MAIL_REQUESTS = parseInt(process.env.MAX_NOVU_INTEGRATION_MAIL_REQUESTS || '300', 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static MAX_NOVU_INTEGRATION_MAIL_REQUESTS = parseInt(process.env.MAX_NOVU_INTEGRATION_MAIL_REQUESTS || '300', 10);
const DEFAULT_INTEGRATION_LIMIT = 300;
...
static MAX_NOVU_INTEGRATION_MAIL_REQUESTS = parseInt(process.env.MAX_NOVU_INTEGRATION_MAIL_REQUESTS || DEFAULT_INTEGRATION_LIMIT, 10);

Also because it looks like we will need it elsewhere (for example tests) so potentially would need to be moved to a configuration file.

@davidsoderberg davidsoderberg marked this pull request as ready for review January 16, 2023 08:22
@davidsoderberg davidsoderberg merged commit 91dae56 into next Jan 16, 2023
@davidsoderberg davidsoderberg deleted the novu-integration-limits-endpoint branch January 16, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants