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

wr-419: Display notification about New articles written by followed authors on all pages #451

Conversation

PablloZz
Copy link
Collaborator

image
image
image

@@ -40,8 +47,24 @@ const useArticlesFeedRoom = (): void => {
articlesSocket?.on(
NEW_ARTICLE,
(newArticle: ArticleSocketEventPayload[typeof NEW_ARTICLE]) => {
if (location.pathname === AppRoute.ARTICLES) {
void dispatch(articleActions.addArticle(newArticle));
if (user?.id === newArticle.socketUserId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure why we need to send socketUserId in the event's payload, can't it be
if (user?.id === newArticle.article.id) return ? Maybe I misunderstood something 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case we only check the author of the article, but on the server we loop through all the sockets in the namespace and send to each user info about each socket on every loop cycle (for example if we have 5 sockets each user will get 5 payloads), so if only one user follows the author, every user will be notified of this publication due this one payload from that user.

Copy link
Collaborator

@artemmatiushenko1 artemmatiushenko1 Sep 29, 2023

Choose a reason for hiding this comment

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

Maybe you should cooperate with @canterbery, your tasks seem to be related #458

@andanf-e andanf-e merged commit c09ac66 into development Sep 30, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To Be Tested
Development

Successfully merging this pull request may close these issues.

3 participants