feat(contact-point): fetches contact point on the backend for edit#1422
feat(contact-point): fetches contact point on the backend for edit#1422ygrishajev merged 2 commits intomainfrom
Conversation
WalkthroughThis update introduces server-side fetching and validation for the contact point edit page, ensuring authenticated access and feature flag checks. It adds a dedicated notifications API client for server-side use, updates OpenAPI and SDK types to support optional Authorization headers, and improves documentation to reflect these changes across the stack. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Next.js Server
participant Auth0
participant FeatureFlagService
participant NotificationsAPI
User->>Next.js Server: Request /alerts/contact-points/[id]
Next.js Server->>FeatureFlagService: Check "alerts" feature flag
FeatureFlagService-->>Next.js Server: Flag enabled/disabled
Next.js Server->>Auth0: Get user session
Auth0-->>Next.js Server: User session or null
Next.js Server->>NotificationsAPI: Fetch contact point by id (with Authorization)
NotificationsAPI-->>Next.js Server: Contact point data or not found
Next.js Server-->>User: Render page with contact point or 404
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1422 +/- ##
==========================================
- Coverage 37.11% 37.06% -0.06%
==========================================
Files 830 831 +1
Lines 19777 19799 +22
Branches 3610 3639 +29
==========================================
- Hits 7341 7339 -2
- Misses 11708 12138 +430
+ Partials 728 322 -406
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ygrishajev
force-pushed
the
feature/contact-point-ssr
branch
from
4c2cbe2 to
ee780d5
Compare
![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 (2)
apps/deploy-web/src/services/server-side-notifications-api/server-side-notifications-api.service.tsx (1)
1-9: LGTM! Clean server-side API client implementation.The implementation correctly creates a server-side notifications API client using the established patterns:
- Proper imports from the notifications SDK and OpenAPI libraries
- Uses existing server environment configuration for the base URL
- Simple, focused responsibility with clear exports
Consider using
.tsextension instead of.tsxsince this file doesn't contain any JSX:- server-side-notifications-api.service.tsx + server-side-notifications-api.service.tsThis would better reflect the file's content and purpose as a pure service module.
apps/deploy-web/src/components/alerts/EditContactPointPage.tsx (1)
13-14: Consider removing debug logging before production.The console.log statement is useful for development but should be removed or replaced with proper logging before production deployment.
- // TODO: use ssr contact point - console.log("DEBUG contactPoint", props.contactPoint); + // TODO: use ssr contact point
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/src/components/alerts/EditContactPointPage.tsx(1 hunks)apps/deploy-web/src/pages/alerts/contact-points/[id]/index.tsx(1 hunks)apps/deploy-web/src/services/server-side-notifications-api/server-side-notifications-api.service.tsx(1 hunks)apps/notifications/src/common/controllers/worker-healthz/worker-healthz.controller.ts(1 hunks)apps/notifications/src/interfaces/rest/controllers/healthz/healthz.controller.ts(1 hunks)apps/notifications/src/interfaces/rest/decorators/http-validate/http-validate.decorator.ts(2 hunks)apps/notifications/swagger/swagger.json(9 hunks)packages/react-query-sdk/src/notifications/schema.ts(9 hunks)packages/react-query-sdk/src/notifications/services/V1Service.ts(73 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/notifications/src/interfaces/rest/controllers/healthz/healthz.controller.ts (1)
apps/notifications/src/common/controllers/worker-healthz/worker-healthz.controller.ts (1)
ApiExcludeController(8-26)
apps/notifications/src/common/controllers/worker-healthz/worker-healthz.controller.ts (1)
apps/notifications/src/interfaces/rest/controllers/healthz/healthz.controller.ts (1)
ApiExcludeController(8-26)
apps/deploy-web/src/components/alerts/EditContactPointPage.tsx (1)
packages/react-query-sdk/src/notifications/schema.ts (1)
components(73-396)
apps/deploy-web/src/services/server-side-notifications-api/server-side-notifications-api.service.tsx (1)
apps/stats-web/src/config/server-env.config.ts (1)
serverEnvConfig(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate-notifications
- GitHub Check: test-deploy-web-build
🔇 Additional comments (22)
apps/notifications/src/common/controllers/worker-healthz/worker-healthz.controller.ts (1)
2-2: LGTM! Consistent exclusion of health check endpoint from API docs.The addition of
@ApiExcludeController()properly excludes this health check controller from Swagger documentation, which is appropriate since health endpoints are typically internal and not part of the public API. This change is consistent with the similar update made to theHealthzController.Also applies to: 8-8
apps/notifications/src/interfaces/rest/controllers/healthz/healthz.controller.ts (1)
2-2: LGTM! Properly excludes health check endpoint from Swagger docs.The
@ApiExcludeController()decorator correctly excludes this health check controller from API documentation. This is consistent with the same change applied toWorkerHealthzControllerand aligns with best practices since health endpoints are internal infrastructure concerns rather than public API features.Also applies to: 8-8
apps/notifications/src/interfaces/rest/decorators/http-validate/http-validate.decorator.ts (1)
2-2: LGTM! Proper implementation of optional Authorization header support.The addition of the
ApiHeaderdecorator for the optional Authorization header is well-implemented:
- Correctly imported
ApiHeaderfrom@nestjs/swagger- Header is properly configured as optional with
required: false- Aligns with the PR's broader objective of adding authorization support to the notifications API
This change will automatically apply the Authorization header documentation to all endpoints using the
ValidateHttpdecorator.Also applies to: 58-64
apps/deploy-web/src/components/alerts/EditContactPointPage.tsx (2)
2-2: Good addition of strong typing support.The import of notification SDK types will enable proper type checking for contact point data.
8-10: Well-defined Props interface.The Props type correctly uses the ContactPointOutput schema from the notifications SDK, ensuring type safety for the contact point data.
packages/react-query-sdk/src/notifications/schema.ts (1)
402-404: Authorization header support added correctly.The auto-generated schema correctly reflects the updated API specification that now supports optional Authorization headers for all alert and contact point operations. This enables authenticated requests as intended by the PR objectives.
Also applies to: 464-466, 524-526, 584-586, 653-655, 711-713, 773-775, 842-844, 911-913
apps/notifications/swagger/swagger.json (1)
7-16: LGTM: Consistent Authorization header support added.The addition of optional Authorization header parameters across all endpoints is implemented consistently and follows OpenAPI best practices. This enables flexible authentication while maintaining backward compatibility with existing clients.
Also applies to: 95-103, 171-179, 257-265, 327-336, 423-431, 501-509, 587-595, 683-691
apps/deploy-web/src/pages/alerts/contact-points/[id]/index.tsx (5)
1-6: LGTM: Well-structured imports with proper type imports.The imports are well-organized and use appropriate type imports for better tree-shaking. All dependencies are necessary for the server-side implementation.
Also applies to: 8-11
15-19: LGTM: Proper route parameter validation.Using Zod to validate the
idparameter as a UUID is excellent practice for type safety and runtime validation of route parameters.
21-27: LGTM: Strong typing with reusable constants.Using the notifications SDK types ensures type safety, and the
NOT_FOUNDconstant promotes consistent 404 responses across the function.
32-42: LGTM: Proper feature flag and authentication validation.The implementation correctly:
- Validates feature flag availability
- Ensures user authentication via Auth0 session
- Returns consistent 404 responses to avoid information leakage
44-64: LGTM: Secure API call with proper error handling.The implementation demonstrates excellent practices:
- Uses Bearer token authorization from the session
- Validates API response data exists before proceeding
- Returns properly typed props matching the component interface
- Maintains consistent 404 error handling for missing data
packages/react-query-sdk/src/notifications/services/V1Service.ts (10)
74-89: Documentation examples consistently updated with Authorization header usage.The JSDoc examples for
createAlert.useMutationnow properly demonstrate how to include the Authorization header, which aligns with the newly added optional Authorization parameter support. The examples show both predefined parameters and inline parameter usage patterns.Also applies to: 103-118, 136-141, 180-185
226-230: Authorization header examples added consistently to getAlert service methods.All query-related documentation examples for
getAlertnow include Authorization header usage, coveringuseQuery,useInfiniteQuery,useIsFetching,useQueries, anduseSuspenseQuerymethods. This provides comprehensive guidance for developers.Also applies to: 247-250, 381-385, 419-423, 463-467, 487-490, 512-515, 520-523, 548-551, 568-571, 591-594, 631-634, 656-659, 664-667, 691-694
722-726: patchAlert examples updated with Authorization header support.The documentation examples for
patchAlert.useMutationand related methods now consistently demonstrate Authorization header usage, maintaining the same pattern established for other service methods.Also applies to: 736-739, 796-800, 843-847
883-886: deleteAlert examples updated with Authorization header support.The
deleteAlertservice method documentation examples now include Authorization header usage, completing the pattern for all alert-related operations.Also applies to: 897-900, 957-961, 1004-1008
1043-1047: createContactPoint examples updated with Authorization header support.All
createContactPointmutation-related examples now demonstrate proper Authorization header usage, includinguseMutation,useIsMutating, anduseMutationStatemethods.Also applies to: 1054-1058, 1079-1083, 1090-1094, 1119-1124, 1177-1182
1246-1250: getContactPoints examples comprehensively updated with Authorization header support.All query-related documentation examples for
getContactPointsnow include Authorization header usage across all hook variants (useQuery,useInfiniteQuery,useIsFetching,useQueries,useSuspenseInfiniteQuery,useSuspenseQueries,useSuspenseQuery).Also applies to: 1278-1282, 1448-1452, 1491-1495, 1543-1547, 1567-1571, 1592-1595, 1632-1636, 1664-1668, 1692-1696, 1736-1739, 1761-1764, 1770-1773, 1800-1803
1845-1849: getContactPoint examples updated with Authorization header support.All
getContactPointquery-related examples now consistently demonstrate Authorization header usage across all hook variants, maintaining the established documentation pattern.Also applies to: 1873-1877, 2041-2045, 2083-2087, 2131-2135, 2155-2158, 2163-2166, 2180-2183, 2188-2191, 2216-2220, 2244-2248, 2270-2274, 2310-2313, 2318-2321, 2335-2338, 2343-2346, 2370-2374
2406-2410: patchContactPoint examples updated with Authorization header support.The
patchContactPointmutation examples now include proper Authorization header usage in the documentation, completing the pattern for contact point operations.Also applies to: 2420-2424, 2448-2452, 2462-2466, 2494-2498, 2555-2559
2616-2620: deleteContactPoint examples updated with Authorization header support.The final set of examples for
deleteContactPointnow includes Authorization header usage, completing the comprehensive update across all service methods.Also applies to: 2630-2634, 2658-2662, 2672-2676, 2704-2708, 2765-2769
2913-2913: Type aliases updated to reference OpenAPI-generated parameter types.The
CreateAlertParametersandCreateContactPointParameterstype aliases now directly reference the OpenAPI-generated parameter types, which include the newly added optional Authorization header. This ensures type safety and consistency with the underlying API specification.Also applies to: 2962-2962
ygrishajev
force-pushed
the
feature/contact-point-ssr
branch
from
ee780d5 to
e831830
Compare
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes