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

iOS - Use "rich" notifications with avatar and room for new experience HybridApp users #51846

Closed
JmillsExpensify opened this issue Nov 1, 2024 · 15 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Nov 1, 2024

For iOS users with tryNewDot enabled and that use HybridApp, we're not delivering "rich" notifications that include the user's avatar and a clarification of what room the conversation is happening in. Here's two examples, one is a DM and another convo is a "test" discussion happening in a room/workspace chat.

IMG_A1EC932509E8-1

In contract, whenever:

  • A user is on HybridApp
  • And has tryNewDot enabled

Then we should be sending notifications that look like this.
IMG_7285

@JmillsExpensify JmillsExpensify added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Bug Something is broken. Auto assigns a BugZero manager. labels Nov 1, 2024
Copy link

melvin-bot bot commented Nov 1, 2024

Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.

@Julesssss
Copy link
Contributor

This will be a good task for our SWM team.

@arosiclair
Copy link
Contributor

We implemented this for NewDot iOS in #32765 and Android in #37604 so we just need to port over those changes to the HybridApp. It sounds like that might have already been done for Android.

@Julesssss
Copy link
Contributor

Yeah this is working for Android, I'll update the description 👍

@Julesssss Julesssss changed the title Use "rich" notifications with avatar and room for all tryNewDot users with HybridApp iOS - Use "rich" notifications with avatar and room for new experience HybridApp users Nov 1, 2024
@melvin-bot melvin-bot bot added the Overdue label Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

@JmillsExpensify, @Julesssss Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@Julesssss
Copy link
Contributor

iOS notifications are tough to test. Our best bet would be to ask an engineer who has already worked on HybridApp notifications, but the initial SWM HybridApp guys are all working on much more critical tasks at the moment.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 5, 2024
@Julesssss
Copy link
Contributor

We need an internal engineer for this.

I am working on a P/S for making noticiation DevX easier -- I think it makes sense to hold this issue until the tooling is better and use it as motivation for that project.

@melvin-bot melvin-bot bot removed the Overdue label Nov 12, 2024
Copy link

melvin-bot bot commented Nov 15, 2024

@JmillsExpensify @Julesssss this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Nov 15, 2024
@Julesssss Julesssss added Weekly KSv2 and removed Daily KSv2 labels Nov 15, 2024
@melvin-bot melvin-bot bot removed the Overdue label Nov 15, 2024
@Julesssss
Copy link
Contributor

Reducing priority as I don't think this needs a daily update. But DevX improvements for notifications are incoming, and I think align quite closely with #quality -- more details to follow shortly!

@arosiclair
Copy link
Contributor

Starting some work on this today. For iOS we're going to need to

  1. Enable mutable_content flag for HybridApp push notifications like we did for NewDot here
  2. Follow the Airship guide for adding the NSE to our Xcode project
    • The bundle ID will be com.expensify.expensifylite.NotificationServiceExtension
  3. Copy over the NewDot implementation we have for it here
  4. Register the new bundleID and enable comms notification capability in the Apple developer console like we did here

@arosiclair
Copy link
Contributor

arosiclair commented Nov 20, 2024

I made another issue to make Apple developer console changes for this here. Gonna need a deployer to make the changes. I'm blocked on development until that's done.

@melvin-bot melvin-bot bot added the Overdue label Nov 29, 2024
@arosiclair arosiclair added Daily KSv2 and removed Weekly KSv2 labels Dec 2, 2024
@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
@arosiclair
Copy link
Contributor

This is nearly done just need to do final tests and I'll post the PR for review.

@arosiclair
Copy link
Contributor

Out for review!

@arosiclair arosiclair added the Reviewing Has a PR in review label Dec 3, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

@JmillsExpensify, @arosiclair Whoops! This issue is 2 days overdue. Let's get this updated quick!

@arosiclair
Copy link
Contributor

This is all done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
Development

No branches or pull requests

3 participants