App
App copied to clipboard
[$250] Notification displays the `@expensify.sms` domain in the title alongside the phone number.
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: 9.0.73-6 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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: @jayeshmangwani Slack conversation (hyperlinked to channel name): expensify_bugs
Action Performed:
- Log in with a phone number as User A.
- Log in with any other account as User B.
- Send a message from User A to User B.
- On User B's device, the notification includes [
@expensify.sms](mailto:@expensify.sms)` along with the phone number.
Expected Result:
Notification should display only the sender's phone number.
Actual Result:
Notification includes both the @expensify.sms domain and the sender's phone number.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Standalone
- [x] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021866600416959560133
- Upwork Job ID: 1866600416959560133
- Last Price Increase: 2024-12-10
Issue Owner
Current Issue Owner: @parasharrajat
Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Job added to Upwork: https://www.upwork.com/jobs/~021866600416959560133
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)
Edited by proposal-police: This proposal was edited at 2024-12-11 00:24:30 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
- Notification includes both the @expensify.sms domain and the sender's phone number.
What is the root cause of that problem?
- The notification's title is from:
https://github.com/Expensify/App/blob/d9be98df93045f4a05419d60a822d5dee7c44c55/src/libs/Notification/LocalNotification/BrowserNotifications.ts#L102
and we didn't remove the sms domain in here.
What changes do you think we should make in order to solve the problem?
-
In here we should remove the sms domain by using
Str.removeSMSDomain(f.text). -
Similar should be applied in other platform.
-
We can check
Str.isSMSLoginbefore removing sms domain.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
- We can use
LocalePhoneNumber.formatPhoneNumber(f.text)as well.
I believe this can be solved at the backend. It is a backend issue.
:ribbon: :eyes: :ribbon: C+ reviewed
Triggered auto assignment to @rlinoz, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@parasharrajat I believe we can handle this on the frontend, as the utility function removeSMSDomain was created to address a similar issue and is already widely used across the app. What do you think about my proposal here?
I think you are right, let me check.
Imo, this issue originated from backend. Frontend code was not changed since last year.
@parasharrajat Could you help check my comment above, please? Thanks
I've changed to to internal for now. π
Oh wow, I missed that you had already created and merged a PR, thanks @rlinoz!
@parasharrajat do you know how we usually handle these cases where you reviewed proposals but didn't end up reviewing the PR? Are you due any payment here?
@sakluger No payment here. Thanks.
Got it, thanks for confirming.
We can close as soon as the PR is deployed to prod.
PR is in staging
Hmm yeah, I think this should be testable in staging, but not 100% sure.
I just tested in staging but the issue is still encountered.
@rlinoz can you also still reproduce this issue?
Yep I can, @truph01 is correct, I didn't realize the notification title for web is built in the FE, even though @truph01 said exactly that π
The backend PR fixed a different issue in the end.
Assigning you to the issue
Current assignee @parasharrajat is eligible for the External assigner, not assigning anyone new.
π£ @truph01 π 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 π
@parasharrajat PR is ready
@rlinoz I encountered the same issue on native https://github.com/Expensify/App/pull/54275#issuecomment-2555261230. We still need backend solution for it.
Ah weird, it works for me in staging at least, might be something related to the number formatting maybe? I will try in dev with the same number you tested
@rlinoz Any updates for us?
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.81-6 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/54275
If no regressions arise, payment will be issued on 2025-01-15. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @parasharrajat requires payment through NewDot Manual Requests
- @truph01 requires payment automatic offer (Contributor)
- @jayeshmangwani requires payment through NewDot Manual Requests
@parasharrajat @sakluger @parasharrajat The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]