-
Notifications
You must be signed in to change notification settings - Fork 361
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
fix: [M3-8409] - Community Notifications in Event Messages v2 Refactor #10742
fix: [M3-8409] - Community Notifications in Event Messages v2 Refactor #10742
Conversation
Coverage Report: ✅ |
f5bbe81
to
2cee22e
Compare
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.
Thanks for the follow up, @pmakode-akamai.
Confirmed that this is looking good for v2 community_like
events. I'd forgotten that we'd added a default of Linode
- works well here. We'll just want a unit test for the new util.
I'm tempted to say that we should add community_question_reply
and community_mention
events on the ACTIONS_WITHOUT_USERNAMES
too, since those are events generated by people other than the user, and Linode is informing the user of them - not sure if @jaalah-akamai or others have thoughts on that.
packages/manager/src/features/NotificationCenter/NotificationData/RenderEventV2.tsx
Outdated
Show resolved
Hide resolved
2cee22e
to
910f86d
Compare
@mjac0bs As discussed with Jaalah before, we wanted to exclude usernames from What are your thoughts on this @jaalah-akamai? As suggested by @mjac0bs, Should the username be excluded from |
Yea I agree with @mjac0bs |
Okay. Excluded usernames from |
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.
The update to include all community events as "without username" looks good on the v2 end.
Minor: I checked for regressions with v1 and noticed that there's a tiny formatting issue with community_question_reply
- a space before the period because we seem to be formatting links that way, which is also gets rid of the quotes in the v1 community_question_reply
. That message we define is: There has been a reply to your thread "${e.entity.label}".
It would more closely match the community_mention
and fix the space issue if we tweaked it to be: There has been a reply to your thread: ${e.entity.label}.
- the period would get replaced by the space. That might be the easiest fix, because although I'm not sure when we plan on turning Events v2 on in prod, my thought is that v1 won't be around much longer.
aefbb4b
to
3c734db
Compare
3c734db
to
2900609
Compare
Description 📝
Improve Community Like Notifications in Event Messages v2 Refactor.
When an answer/question is "liked" on the Community Questions site, the person wrote that answer/question is sent a notification in Cloud Manager. However, the syntax for that notification doesn't really make sense in Event Messages v2:
Note
Default 'Linode' is kept as it is because it was already defined as a fallback when the username is
null
in Event Messages v2.Changes 🔄
In Event Messages v2:
Target release date 🗓️
8/19
Preview 📷
How to test 🧪
Reproduction steps
With the Event Messages v2 flag ON locally:
Verification steps
ACTIONS_WITHOUT_USERNAMES
or whenusername
is null.ACTIONS_WITHOUT_USERNAMES
or whenusername
is null.As an Author I have considered 🤔
Check all that apply