-
Notifications
You must be signed in to change notification settings - Fork 109
Clean up of the notification concept page #886
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe documentation for the Novu notification lifecycle was revised for clarity and conciseness. Explanations of events, workflows, notifications, and messages were rephrased and reorganized, with redundant sections removed and references to related APIs and documentation updated. No changes were made to any exported or public code entities. Changes
Sequence Diagram(s)Possibly related PRs
Suggested reviewers
Poem
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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
content/docs/platform/concepts/notifications.mdx (2)
23-26
: Fix comma-splice and improve flow of the “Event” definitionThe sentence is currently a comma-splice and reads a bit heavy. Splitting it keeps the pace consistent with the rest of the page.
-The notification lifecycle starts with an event, this event represents something meaningful that has happened in your application, such as a new comment, user sign-up, or password reset request. +The notification lifecycle starts with an **event**. An event represents something meaningful that has happened in your application—such as a new comment, user sign-up, or password-reset request.
27-32
: Minor consistency tweak: terminate introductory line with a colonFor bullet-list intros elsewhere you end with a colon; do the same here for uniformity.
-Each event in Novu contains +Each event in Novu contains:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/concepts/notifications/notification-life-cycle.png
is excluded by!**/*.png
📒 Files selected for processing (1)
content/docs/platform/concepts/notifications.mdx
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
content/docs/platform/concepts/notifications.mdx
[uncategorized] ~35-~35: Possible missing comma found.
Context: ...errides or metadata ### Workflow This event that was triggered in your application,...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~35-~35: Possible missing comma found.
Context: ...be used to send the notification to the user and also the format in which the notifi...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...various channels. This is covered in depth in the [Workflows c...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Lint
🔇 Additional comments (2)
content/docs/platform/concepts/notifications.mdx (2)
35-37
: Re-phrase to remove the dangling introductory clause and missing comma-This event that was triggered in your application, in turn triggers the workflow(s) attached to it. This workflow determines the delivery channels to be used to send the notification to the user and also the format in which the notification content will be displayed in the various channels. +When Novu receives the event, it triggers the attached workflow(s). +The workflow decides which delivery channels to use and how the notification content should be formatted for each channel.This eliminates the dangling “This event that was triggered…” construction, removes the comma splice, and shortens the second sentence for clarity.
[tag: suggest_nitpick]
53-61
: Verify the API links & avoid duplicationBoth the Notification and Message sections link to
/api-reference/notifications/list-all-events
.
The second link should probably point to the Message API (e.g./api-reference/messages/*
). Please confirm the correct endpoint and update accordingly to prevent user confusion.
✅ Deploy Preview for docs-novu ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
content/docs/platform/concepts/notifications.mdx (2)
23-26
: Break up the comma-splice for readabilityCurrent sentence contains two independent clauses joined only by a comma. Split into two sentences (or use an em-dash/semicolon) to avoid a comma splice and improve clarity.
-The notification lifecycle starts with an event, this event represents something meaningful that has happened in your application, such as a new comment, user sign-up, or password reset request. +The notification lifecycle starts with an event. This event represents something meaningful that has happened in your application—such as a new comment, user sign-up, or password-reset request.
35-38
: Streamline wording and remove superfluous comma
- Introductory “This event that was triggered…” is clunky and the comma after application is unnecessary.
- Two consecutive sentences start with This—varying the structure reads better.
-This event that was triggered in your application, in turn triggers the workflow(s) attached to it. This workflow determines the delivery channels to be used to send the notification to the user and also the format in which the notification content will be displayed in the various channels. +When the event is triggered in your application, it in turn activates the attached workflow(s). The workflow decides which delivery channels to use and how the notification content is formatted for each channel.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
content/docs/platform/concepts/notifications.mdx
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
content/docs/platform/concepts/notifications.mdx
[uncategorized] ~35-~35: Possible missing comma found.
Context: ...errides or metadata ### Workflow This event that was triggered in your application,...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~35-~35: Possible missing comma found.
Context: ...be used to send the notification to the user and also the format in which the notifi...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...various channels. This is covered in depth in the [Workflows c...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - docs-novu
- GitHub Check: Header rules - docs-novu
- GitHub Check: Pages changed - docs-novu
- GitHub Check: Build and Lint
🔇 Additional comments (2)
content/docs/platform/concepts/notifications.mdx (2)
53-54
: Verify Message-API link targetBoth Notification and Message sections point to
notifications/list-all-events
. The second link should presumably reference a Message-specific endpoint (e.g.,/messages
). Please confirm and adjust if needed to avoid misleading users.
57-57
: “phase” typo corrected — good catchThe earlier typo (
phrase
) has been fixed. ✅
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.
- revisit this doc
- mention notification event
- since we have this page only once, try to have crisp but all information related to event on this page
These events are how your application communicates to Novu that it is time to send a particular notification to certain subscribers. | ||
|
||
Each event in Novu contains: | ||
- A name that maps to a specific workflow (`user_signed_up`) |
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.
Please rephrase this line,
A name that maps to
- When and how messages should be sent | ||
|
||
The workflow is where message personalization, conditional logic, and multi-channel orchestration happens. | ||
This event that was triggered in your application, in turn triggers the workflow(s) attached to it. This workflow determines the delivery channels to be used to send the notification to the user and also the format in which the notification content will be displayed in the various channels. |
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.
please revisit this
in my opinion, bullet points are more easy to comprehend by user
Please take a look on above comments |
Summary by CodeRabbit