feat: Add support for fetching all notification templates#57
feat: Add support for fetching all notification templates#57VaishnaviSidral wants to merge 3 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR makes search filter parameters optional in the notification events module. The ChangesOptional Search Filters
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the search functionality for notification events by making the search filters optional. Specifically, it modifies the SearchDto and SearchFilterDto to allow for missing context and filter objects, and updates the NotificationEventsService to handle these optional parameters when building database queries. A review comment correctly identifies a validation issue where the context property in SearchDto is missing the @IsOptional() decorator; without this, class-validator will still enforce @IsString() and @IsNotEmpty() even if the field is omitted in the request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/modules/notification_events/dto/searchTemplateType.dto.ts (1)
22-24: ⚡ Quick winUse
@ValidateNested()withouteach: truefor a single nested DTO.The
each: trueoption is intended for validating elements within collections (arrays, sets, maps). Sincefiltersis a singleSearchDtoobject, not an array, theeach: trueparameter should be removed. This makes the validator behavior explicit and correct.Proposed fix
-@ValidateNested({ each: true }) +@ValidateNested() `@Type`(() => SearchDto) filters?: SearchDto🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/notification_events/dto/searchTemplateType.dto.ts` around lines 22 - 24, The `@ValidateNested` decorator on the filters property is incorrectly using each: true for a single nested DTO; update the validation on filters (typed as SearchDto) by removing the each: true option so that `@ValidateNested`() is applied without each: true and keep the existing `@Type`(() => SearchDto) decorator to ensure proper transformation and nested validation of the SearchDto instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/notification_events/dto/searchTemplateType.dto.ts`:
- Around line 7-10: The context property on the DTO is currently decorated with
`@IsString`() and `@IsNotEmpty`(), which causes validation to fail when context is
omitted; add the `@IsOptional`() decorator to the context field in the
SearchTemplateType DTO (the property named context in the DTO class in
searchTemplateType.dto.ts) so that `@IsNotEmpty`() only runs when a value is
present and requests without a context are accepted.
---
Nitpick comments:
In `@src/modules/notification_events/dto/searchTemplateType.dto.ts`:
- Around line 22-24: The `@ValidateNested` decorator on the filters property is
incorrectly using each: true for a single nested DTO; update the validation on
filters (typed as SearchDto) by removing the each: true option so that
`@ValidateNested`() is applied without each: true and keep the existing `@Type`(()
=> SearchDto) decorator to ensure proper transformation and nested validation of
the SearchDto instance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35cc2239-31f2-45fc-9381-2e67bdcfe251
📒 Files selected for processing (2)
src/modules/notification_events/dto/searchTemplateType.dto.tssrc/modules/notification_events/notification_events.service.ts
|
1 participant
No description provided.