HIGH: 'Chat report' title shows for thread comment push notifications

[Julesssss]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

• Within a room or chat, send a message as userA
• As userB, reply within the thread
• As userA, look at the push notification that you receive

Expected Result:

• The notification title should be <original_message>

Actual Result:

• The notification title is Chat Report

Room	Thread

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.73-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Issue reported by: ME
Slack conversation: N/A

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a906e67be084d98b
• Upwork Job ID: 1706164911894806528
• Last Price Increase: 2023-09-25

Issue Owner

Current Issue Owner: @Julesssss

[melvin-bot]

Triggered auto assignment to @trjExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a906e67be084d98b

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @narefyev91 (Internal)

[Julesssss]

I'm going to ignore iOS notifications for now, as they don't even show the room/group title. I'll take the question of how iOS notifications should look to #design:

🤮🤮🤮🤮

[trjExpensify]

Are you working on this Jules, yeah?

[Julesssss]

Are you working on this Jules, yeah?

Yeah.

[Julesssss]

I need to do the following:

• Figure out where we are setting the NVP_PARENT_REPORT_ACTION_ID rNVP
• In the same place, use Report::setNameValuePair(db, finalReportID, Report::NVP_PARENT_REPORT_ID, to_string(parentReportID)); to persist the parentReportActionID to the report
• This will allow me to retrieve the parentReport from within the push notification logic, and then FINALLY i will be able to get the thread message...

[Julesssss]

On second thoughts, we'll need a new auth function to lookup the parentReportID...

• We'll need to do create a new Auth function for retrieving the parent report
• I'll use Auth::getParentReportDetails to get the report

[Julesssss]

Made progress today, but I'm holding to work on a wave 4 issue.

Branches in Web-E & Auth: jules-implementGetParentReportDetails

[Julesssss]

Held for a few weeks while I focus on wave projects.

[Julesssss]

Still held

[Julesssss]

Still held on wave issues

[Julesssss]

Same as above.

[Julesssss]

I took a look at this again recently, and I have a WIP PR where I'm working on the solution.

[quinthar]

Let's add this as HIGH to VIP-VSB. @Julesssss can you give an ETA for completion?

Also, what is the final proposal? I don't think it should say Thread: ... -- we don't use that anywhere. Let's just copy the same exact subject line that we use for emails, explained here: #33579 (comment)

[Julesssss]

ETA a few days, as I'm close to finishing my current wave 5 issue.

Also, what is the final proposal? I don't think it should say Thread: ... -- we don't use that anywhere.

Ah that was an old proposal, updated. We'll match web/Desktop

[Julesssss]

I made progress on this today. I'll submit the draft PR tomorrow after I make some final fixes and fully test it.

[Julesssss]

Today I fixed CI issues and tested the solution. All is well except for thread comments made against unpaid IOU requests.

I need to detect the difference between these reportActions and then find a different solution for retrieving the thread title.

[Julesssss]

Yesterday I had to switch focus to handle a fire.

Today I am holding temporarily as we might change how notification delivery works.

[quinthar]

@rlinoz is handling the email portion, @Julesssss can you take the push notification part?

[quinthar]

Moving to CRITICAL as this is really just an offshot.

[Julesssss]

can you take the push notification part?

@quinthar no problem.

The original issue is fixed, but is now blocked (issue) because a recent PR broke the push notification header/title. I'm working on a separate fix for that today.

[Julesssss]

Debugging is extremely slow here as I'm waiting 30 minutes for every build to compile...

I have a couple of potential causes but will need more time tomorrow to confirm them before I can work on a solution.

[Julesssss]

End of day update: The original Web-E PR almost ready for review. I just need to fix a flakey test failure tomorrow.

[Julesssss]

Still blocked. The flakey test failure is widespread. We're looking into solutions.

[Julesssss]

PR is now in review, the flakey test has been resolved.

[Julesssss]

Merged, awaiting deployment to prod

[quinthar]

Is this done? Can we close it?

[Julesssss]

Is this done? Can we close it?

Yep, closing.