[HOLD for payment 2024-03-06] [$500] Sounds for receiving message and for being tagged somewhere

[m-natarajan]

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

---

Version Number: 1.4.41-2
Reproducible in staging?: y
Reproducible in production?: new feature
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @mountiny
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707866137747939

Action Performed:

1. Send a message mentioning some one else

Expected Result:

Should not receive sound notification. This should only happen when you are tagged (directly or (at)here)

Actual Result:

you still receive the sound effect as if you would be mentioned

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.2730.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0158c6c5402e5ea2cd
• Upwork Job ID: 1757878333789503488
• Last Price Increase: 2024-02-14
• Automatic offers:
• cubuspl42 | Reviewer | 0
• rayane-djouah | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0158c6c5402e5ea2cd

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @cubuspl42 (External)

[shawnborton]

Thanks! Again cc @kirillzyusko and @hannojg for visibility

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[rayane-djouah]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When a user is mentioned, a sound is triggered. However, the current implementation plays the sound regardless of who is mentioned.

What is the root cause of that problem?

here:

App/src/libs/actions/User.ts

Lines 528 to 531 in 28bcc85

	// mention user
	if ('html' in message && typeof message.html === 'string' && message.html.includes('<mention-user>')) {
	return playSound(SOUNDS.ATTENTION);
	}

the code checks if a message includes a mention without verifying the identity of the mentioned user.

What changes do you think we should make in order to solve the problem?

To address this, we propose modifying the condition as follows:

  // mention user
  if ('html' in message && typeof message.html === 'string' && message.html.includes(`<mention-user>@${currentEmail}</mention-user>`)) {
      return playSound(SOUNDS.ATTENTION);
  }

This adjustment ensures that the sound is played only when the current user is mentioned.

What alternative solutions did you explore? (Optional)

N/A

[kirillzyusko]

@rayane-djouah yes, i think it's a correct solution that you are suggesting 👍

[stephanieelliott]

Hey @cubuspl42 proposal for you to review here when you get a sec!

[cubuspl42]

🎀 👀 🎀

[melvin-bot]

Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[cubuspl42]

@flodnv Would it be possible to add more metadata to this structure returned from the backend? I know that in another place...

App/src/types/onyx/OriginalMessage.ts

Line 174 in 28bcc85

targetAccountIDs?: number[];

...we have access to the account IDs of the mentions.

[flodnv]

Probably, but my plate is full, so either @stephanieelliott or @shawnborton will have to find someone to work on this (it's not clear to me which part of the roadmap this is on)

[shawnborton]

cc @Julesssss - we talked about putting this on the VSB project.

[Julesssss]

cc @Julesssss - we talked about putting this on the VSB project.

Ah, I was using this issue for the native mobile audio bug

[shawnborton]

Happy to do whatever you think is best. What do you think about consolidating these issues into just one follow up issue for the couple of bugs that have popped up?

[shawnborton]

Sounds like we don't want to consolidate all sound issues. But @kirillzyusko curious if you are planning to handle this as a follow up?

[Julesssss]

What do you think about consolidating these issues into just one follow up issue for the couple of bugs that have popped up?

I said this in Slack, but just to clarify: We'll keep the mobile bugs separate as testing mobile complicates the testing of other issues.

I'll help @kirillzyusko to review and test

[rayane-djouah]

still interested to take this issue if my proposal is accepted

[Julesssss]

Thanks @rayane-djouah, lets go with your solution as we don't have time for any backend changes here.

[melvin-bot]

📣 @cubuspl42 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[kirillzyusko]

Sounds like we don't want to consolidate all sound issues

I wouldn't like to fix all sounds-related issues in a single MR, but I'm not agains the idea of creation kind of main issue for sounds, were we will have a references to all bugs (but separate bugs will be tracked separately). I think such approach will give a better understanding of what happens/what is the current status/simplify management.

@Julesssss @shawnborton What do you think? 👀

[shawnborton]

I'd be down with that, what do you think @Julesssss ?

[Julesssss]

wouldn't like to fix all sounds-related issues in a single MR, but I'm not agains the idea of creation kind of main issue for sounds, were we will have a references to all bugs (but separate bugs will be tracked separately)

Yeah, that sounds ideal 👍

I created this issue, please add any issues/bugs you are aware of. Also, @kirillzyusko could please comment on the issue so I can assign you too. thanks

[stephanieelliott]

Fix for this issue is on staging, ended up testing it and it's working great 👍

[stephanieelliott]

This was deployed to prod on 2/28, seems the automation didn't work so am updating manually

[rayane-djouah]

@stephanieelliott friendly reminder for payment. Thank you!

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @rayane-djouah $500 PAID via Upwork
• Contributor+: @cubuspl42 $500 PAID via Upwork

Upwork job is here: https://www.upwork.com/jobs/~0158c6c5402e5ea2cd