Skip to content

feat(react): upsert firstName, lastName, and email on session init#8142

Merged
djabarovgeorge merged 25 commits into
nextfrom
nv-5683-support-of-upserting-subscriber-attributes-with-inbox
Apr 28, 2025
Merged

feat(react): upsert firstName, lastName, and email on session init#8142
djabarovgeorge merged 25 commits into
nextfrom
nv-5683-support-of-upserting-subscriber-attributes-with-inbox

Conversation

@djabarovgeorge
Copy link
Copy Markdown
Contributor

What changed? Why was the change needed?

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

@linear
Copy link
Copy Markdown

linear Bot commented Apr 16, 2025

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 16, 2025

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit 9694143
🔍 Latest deploy log https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/680fcabaae51140008d79825

Copy link
Copy Markdown
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

Wanted to do this for a long time 👏 What are your thoughts on props vs doing a subscriber object prop? @LetItRock wdyt?

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 20, 2025

Open in StackBlitz

@novu/js

npm i https://pkg.pr.new/novuhq/novu/@novu/js@8142

@novu/nextjs

npm i https://pkg.pr.new/novuhq/novu/@novu/nextjs@8142

novu

npm i https://pkg.pr.new/novuhq/novu@8142

@novu/react

npm i https://pkg.pr.new/novuhq/novu/@novu/react@8142

@novu/react-native

npm i https://pkg.pr.new/novuhq/novu/@novu/react-native@8142

commit: 9694143

Comment thread apps/api/src/app/inbox/dtos/subscriber-session-request.dto.ts
Comment thread apps/api/src/app/inbox/usecases/session/session.command.ts Outdated
Comment thread packages/react/src/utils/types.ts
Comment thread packages/js/src/types.ts
}

return data;
return convertedObject;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is needed in order to get the default in the class like we have here

  allowUpdate?: boolean = true;

otherwise the value is undefined.
any objection to this change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That is the correct value we should have here 🤔 @tatarco it seem to be modified here: #7173 is there any reason for this?

Comment thread apps/api/src/app/inbox/e2e/session.e2e.ts Outdated
Comment thread apps/api/src/app/inbox/e2e/session.e2e.ts Outdated
}

return data;
return convertedObject;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That is the correct value we should have here 🤔 @tatarco it seem to be modified here: #7173 is there any reason for this?

Comment thread packages/js/src/types.ts
readonly applicationIdentifier: string;

@IsString()
@IsOptional()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the Inbox set of endpoint without OpenAPI or Speakeasy SDK yet.

preferencesFilter?: PreferencesFilter;
routerPush?: RouterPush;
};
} & (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can export this type from @novu/js and reuse them in react.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i thought about keeping It's simple with fewer types

@djabarovgeorge djabarovgeorge merged commit 117a764 into next Apr 28, 2025
30 checks passed
@djabarovgeorge djabarovgeorge deleted the nv-5683-support-of-upserting-subscriber-attributes-with-inbox branch April 28, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants