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

feat(framework): Add new Inbox properties to step.inApp schema #6075

Merged
merged 24 commits into from
Aug 5, 2024

Conversation

rifont
Copy link
Contributor

@rifont rifont commented Jul 15, 2024

What changed? Why was the change needed?

  • New step.inApp properties are needed to support the inbox component.

Screenshots

Everything specified

export const inAppWorkflow = workflow('in-app-workflow', async ({ step }) => {
  await step.inApp('send-in-app', (controls) => ({
    subject: `Welcome!`,
    body: 'Hello there',
    avatar: 'https://static.vecteezy.com/system/resources/thumbnails/006/487/917/small_2x/man-avatar-icon-free-vector.jpg',
    primaryAction: {
      label: 'Primary',
      url: 'https://google.com',
    },
    secondaryAction: {
      label: 'Secondary',
      url: 'https://google.com',
    }
  }));

Studio Preview
image

Dashboard Preview
image

V1 In-App Delivered Notification
image

V2 Inbox Delivered Notification
image

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Copy link

linear bot commented Jul 15, 2024

@rifont rifont changed the title feat(schemas): add schemas for avatar, action, and subscriber feat(framework): Add step.inApp schema to support new Inbox properties Jul 15, 2024
@rifont rifont changed the title feat(framework): Add step.inApp schema to support new Inbox properties feat(framework): Add new Inbox properties to step.inApp schema Jul 15, 2024
@BiswaViraj
Copy link
Member

/* 👇is this property meant to be specifiable in the Framework? There's some duplication with avatar */

Not sure, if this is clear now, but still want to address here for visibility.
In the example, it does seem like there's some duplication in avatar and actor as we are using user type
But in the scenario, where avatar is a system icon or custom
The actor will be undefined or null.
@rifont

@rifont
Copy link
Contributor Author

rifont commented Jul 16, 2024

/* 👇is this property meant to be specifiable in the Framework? There's some duplication with avatar */

Not sure, if this is clear now, but still want to address here for visibility. In the example, it does seem like there's some duplication in avatar and actor as we are using user type But in the scenario, where avatar is a system icon or custom The actor will be undefined or null. @rifont

Thanks for the update @BiswaViraj . In that case, given that the presence of actor in dependent on the presence of avatar, does it make more sense to more closely co-locate the attributes? I am thinking we can merge the avatar and actor property, so we have:

actor.type==='user'

const inAppRes = await step.inApp('send-in-app', async () => ({
  body: 'Test Body',
  actor: {
    type: 'user',
    subscriberId: 'joe@acme.com', // required when type is user
    firstName: 'Joe', // optional when type is user, overrides default subscriber first name
    lastName: 'Smith', // optional when type is user, overrides default subscriber last name
    avatar: 'https://novu.co/image', // optional when type is user, overrides the default subscriber avatar
  },
}));

actor.type==='custom'

const inAppRes = await step.inApp('send-in-app', async () => ({
  body: 'Test Body',
  actor: {
    type: 'custom',
    avatar: 'https://novu.co/image', // type: URL - required when type is `custom`
  },
}));

actor.type==='system'

const inAppRes = await step.inApp('send-in-app', async () => ({
  body: 'Test Body',
  actor: {
    type: 'system',
    avatar: 'robot', // type: IconName - required when type is system
  },
}));

Wdyt?

@rifont rifont marked this pull request as draft July 19, 2024 15:42
@rifont
Copy link
Contributor Author

rifont commented Jul 19, 2024

Converted to draft as we still need to wire-up the Worker implementation to support these new attributes in Framework.

@rifont rifont self-assigned this Jul 30, 2024
@rifont rifont marked this pull request as ready for review July 30, 2024 15:45
@rifont rifont requested a review from a team July 30, 2024 15:45
@rifont rifont merged commit d98ee52 into next Aug 5, 2024
28 of 31 checks passed
@rifont rifont deleted the nv-4095-polish-the-inbox-dx-in-workflows branch August 5, 2024 07:40
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