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

🐛 Bug Report: @novu/js - Updating the read state of a Notification results in an empty instance #6631

Closed
2 tasks done
gtnsimon opened this issue Oct 4, 2024 · 5 comments · Fixed by #6639
Closed
2 tasks done
Assignees
Labels
bug Something isn't working High priority Created by Linear-GitHub Sync Inbox @novu/js

Comments

@gtnsimon
Copy link

gtnsimon commented Oct 4, 2024

📜 Description

Hello,

With an InApp notification created with the data (https://docs.novu.co/sdks/framework/typescript/steps/inbox) key inside, updating the read state (using notification.read() or notification.unread()) results to an empty Notification instance.

👟 Reproduction steps

Front-end reproduction (with samples Workflows to reproduce the issue): https://stackblitz.com/edit/sb1-gov1vt?file=main.js

// taken from the Stackblitz repro for quick review
import { Novu } from '@novu/js';

const novu = new Novu({
  subscriberId: import.meta.env.VITE_NOVU_SUB_ID,
  applicationIdentifier: import.meta.env.VITE_NOVU_APP_ID,
  backendUrl: 'https://api.novu.co',
  socketUrl: 'https://ws.novu.co',
});

const notifications = await novu.notifications.list();
const [notificationToUpdate] = notifications.data.notifications ?? [];
const updatedNotification = await notificationToUpdate.read();

console.log('updatedNotification', updatedNotification);

👍 Expected behavior

This is a valid Notification instance with data inside:

image

👎 Actual Behavior with Screenshots

This is bad Notification instance with no data inside:

image

Novu version

Novu SaaS

npm version

NA

node version

NA

📃 Provide any additional context for the Bug.

The Novu API response when calling https://api.novu.co/v1/inbox/notifications/<NOTIFICATION_ID>/read returns the following payload:

image

No data key exists and I think this is the root cause. The @novu/client reads the .data key from response.json() to create a Notification which overlaps with Notification.data definition (https://docs.novu.co/sdks/framework/typescript/steps/inbox):

👀 Have you spent some time to check if this bug has been raised before?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

None

@github-actions github-actions bot added the triage label Oct 4, 2024
@mohdishaq786
Copy link

mohdishaq786 commented Oct 5, 2024

@gtnsimon i want to work on this issue can you please assign me this issue

@gtnsimon gtnsimon changed the title 🐛 Bug Report: @novu/js - Updating the read state of an Notification is resulting in an empty instance 🐛 Bug Report: @novu/js - Updating the read state of a Notification results in an empty instance Oct 7, 2024
@jainpawan21
Copy link
Member

@gtnsimon

Thanks for sharing the bug. I can reproduce the issue.

@jainpawan21
Copy link
Member

@gtnsimon i want to work on this issue can you please assign me this issue

@mohdishaq786 We appreciate your interest in contribution. As this is a high priority bug, our team will fix this as early as possible.

@jainpawan21 jainpawan21 added High priority Created by Linear-GitHub Sync bug Something isn't working Inbox @novu/js and removed triage labels Oct 7, 2024
@BiswaViraj BiswaViraj linked a pull request Oct 7, 2024 that will close this issue
@BiswaViraj BiswaViraj self-assigned this Oct 7, 2024
@jainpawan21
Copy link
Member

@gtnsimon

We just fixed this issue. Can you please recheck this? Feel free to update in this thread if you are still seeing the old behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working High priority Created by Linear-GitHub Sync Inbox @novu/js
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants