App
App copied to clipboard
[$500] [MEDIUM] Referral - Referral link is incomplete and missing contact details
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.6.2 Reproducible in staging?: Y Reproducible in production?: Y 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: Applause - Internal Team Slack conversation:
Issue found when executing PR https://github.com/Expensify/App/pull/31827
Action Performed:
- In Safari or Chrome on Mac OSX, go to staging.new.expensify.com
- Create a new user account
- Tap Search
- Tap on the referral banner at the bottom
- Tap Copy referral link
- Paste the link
Expected Result:
The referral link is complete
Actual Result:
The referral link is incomplete. The link is missing contact details This issue happens for some accounts but not all
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/f3fdab66-45be-4b3e-bb72-e8546d445859
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01cdb9a4da6bb420f6
- Upwork Job ID: 1730291484465594368
- Last Price Increase: 2023-11-30
- Automatic offers:
- situchan | Contributor | 28033535
- tienifr | Contributor | 28135729
Issue Owner
Current Issue Owner: @roryabraham
Triggered auto assignment to @conorpendergrast (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Job added to Upwork: https://www.upwork.com/jobs/~01cdb9a4da6bb420f6
Bug0 Triage Checklist (Main S/O)
- [x] This "bug" occurs on a supported platform (ensure
Platforms
in OP are β ) - [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
- [x] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
- [x] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
- [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (External
)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Referral link is showing empty email address when we generate referral link
What is the root cause of that problem?
On the referral page src/pages/ReferralDetailsPage.js Page when we are generating the referral url using the generateReferralURL
function
https://github.com/Expensify/App/blob/56a368b07233a78d0a3603b652798d1d86821885/src/pages/ReferralDetailsPage.js#L57-L59
we are using the account.primaryLogin
as a email address and sometimes (when a single email added as a contact method) we has empty primaryLogin value
https://github.com/Expensify/App/blob/56a368b07233a78d0a3603b652798d1d86821885/src/pages/ReferralDetailsPage.js#L95
What changes do you think we should make in order to solve the problem?
Instead of the account.primaryLogin
we should use the session.email
or we can use the account.primaryLogin || session.email
as a email address here
https://github.com/Expensify/App/blob/56a368b07233a78d0a3603b652798d1d86821885/src/pages/ReferralDetailsPage.js#L95
urlToCopy={generateReferralURL(account.primaryLogin || session.email)}
What alternative solutions did you explore? (Optional)
N/A
Proposal
Please re-state the problem that we are trying to solve in this issue.
The referral link is incomplete. The link is missing contact details This issue happens for some accounts but not all
What is the root cause of that problem?
- There's a back-end issue where
primaryLogin
is not added first time new account created but after logout and re-login, it's added. This has been discussed extensively in this issue.
We decided to fix in the back-end but due to low capacity, we did not fix it there.
So in here, the primaryLogin is empty, causing the issue
- There's also another issue where when setting another contact method as default here, the
primaryLogin
is not updated in optimistic data. This causes theprimaryLogin
to be outdated in many different places that use it, when we update the default contact method.
What changes do you think we should make in order to solve the problem?
- We should fix this properly in the back-end, if we're still low on capacity, we can optimistically update it with
session.email
in the relevant login APIs. - Update
primaryLogin
tonewDefaultContactMethod
in optimistic data (and restore in failure data) when updating the default contact method
What alternative solutions did you explore? (Optional)
We can consider removing the use of primaryLogin
everywhere and replace by session.email
, but I'm not sure if there's any hidden regression.
We cannot just fix it in the one place of generateReferralURL
since all the bug cases of the 2nd problem will remain.
Proposal
Please re-state the problem that we are trying to solve in this issue.
The email details is not being populated to the copied shareable url for users that recently joined the app
What is the root cause of that problem?
I don't think there's a need to fix on the service side, this should be addressed on the FE app.
The root cause lies on the ReferralDetailsPage
component.
https://github.com/Expensify/App/blob/cc4add5ba055b1d88ef246bf6289092a1344afc5/src/pages/ReferralDetailsPage.js#L57-L59
And how is being called below with the primaryLogin, so it's assuming it's not empty: https://github.com/Expensify/App/blob/cc4add5ba055b1d88ef246bf6289092a1344afc5/src/pages/ReferralDetailsPage.js#L92-L95
What changes do you think we should make in order to solve the problem?
We should provide the credentials stored on Onyx, these hold the credentials the new user just entered to join the app, and on top of that, we should validate whether the primaryLogin
at the account level is not empty, if it is, then we should be able to use the login
at the credentials
level which holds the primary login method the user recently entered.
What alternative solutions did you explore? (Optional)
We could retrieve a user's session email through Onyx as well
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Are there more specific reproduction steps that you were able to work out in developing your proposals @jayeshmangwani, @tienifr or @chiItepin? I haven't been able to reproduce through the steps above, but based on @tienifr details it seems like it might be:
- In Safari or Chrome on Mac OSX, go to staging.new.expensify.com
- Create a new user account
- Tap Search
- Tap on the referral banner at the bottom
- Tap Copy referral link
- Paste the link
Is that correct?
@conorpendergrast Yes, that's exactly right, it only happens for recently created accounts. This explains why the "credentials" entity does have the "login" set in the onyx store
Taking this over based on @ntdiary's request
@jayeshmangwani's proposal looks good to me. There was similar issue in the past and we decided to use session.email instead of fixing in backend. π π π C+ reviewed
Triggered auto assignment to @tgolen, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
There was similar https://github.com/Expensify/App/issues/19366 in the past and we decided to https://github.com/Expensify/App/pull/28164 instead of fixing in backend. π π π C+ reviewed
@situchan Actually we decided to fix in the back-end but due to low capacity, we did not fix it there but use the front-end fix as a workaround instead.
Front-end fix is not fixing the root cause so I think we should fix in back-end here.
cc @tgolen
Maybe @roryabraham has better context
Oof, that's a long issue. It would be great if you had a TLDR @roryabraham.
@tgolen, @conorpendergrast Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
π£ @situchan π 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 π
@situchan assigned to you as C+
I don't think using session.email
is workaround.
Waiting for @roryabraham's confirmation either to use session.email
or fix in backend.
Rory is still on jury duty for today and tomorrow. I think that's fine. This isn't super urgent.
@tgolen @conorpendergrast @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
The back-end "fix" for that issue was implemented, it just ... didn't actually fix the issue.
Sorry if I missed this, but what precisely do we think is the root cause?
@roryabraham Thanks for jumping in!
Sorry if I missed this, but what precisely do we think is the root cause?
Can you check the root cause section of my proposal, the back-end is not returning the primaryLogin
after the account is first time created, but after logged out and logged in, it's returned.
Rory is OoO for jury duty again.
@roryabraham I'll assign you for follow-up when you're back!
@tgolen @conorpendergrast @roryabraham @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!
Rory is back! Assigning him for follow-up
@tgolen, @conorpendergrast, @roryabraham, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Had deja vu creating this PR, but verified that it fixes the issue. The previous implementation of the same thing might have been accidentally removed in a bad merge resolution or something.
Issue not reproducible during KI retests. (First week)
@conorpendergrast, @roryabraham, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!