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

[$500] [MEDIUM] Referral - Referral link is incomplete and missing contact details #32292

Closed
2 of 6 tasks
lanitochka17 opened this issue Nov 30, 2023 · 93 comments
Closed
2 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 30, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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 #31827

Action Performed:

  1. In Safari or Chrome on Mac OSX, go to staging.new.expensify.com
  2. Create a new user account
  3. Tap Search
  4. Tap on the referral banner at the bottom
  5. Tap Copy referral link
  6. 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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6296290_1701365730750.20231201_013259.mp4

View all open jobs on GitHub

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 OwnerCurrent Issue Owner: @miljakljajic
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 30, 2023
@melvin-bot melvin-bot bot changed the title Referral - Referral link is incomplete and missing contact details [$500] Referral - Referral link is incomplete and missing contact details Nov 30, 2023
Copy link

melvin-bot bot commented Nov 30, 2023

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

Copy link

melvin-bot bot commented Nov 30, 2023

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

Copy link

melvin-bot bot commented Nov 30, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • 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
  • 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.
  • 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.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 30, 2023
Copy link

melvin-bot bot commented Nov 30, 2023

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

@jayeshmangwani
Copy link
Contributor

jayeshmangwani commented Nov 30, 2023

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

function generateReferralURL(email) {
return `${CONST.REFERRAL_PROGRAM.LINK}/?thanks=${encodeURIComponent(email)}`;
}

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
urlToCopy={generateReferralURL(account.primaryLogin)}

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

urlToCopy={generateReferralURL(account.primaryLogin)}

urlToCopy={generateReferralURL(account.primaryLogin || session.email)}

What alternative solutions did you explore? (Optional)

N/A

@tienifr
Copy link
Contributor

tienifr commented Dec 1, 2023

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?

  1. 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

  1. There's also another issue where when setting another contact method as default here, the primaryLogin is not updated in optimistic data. This causes the primaryLogin 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?

  1. 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.
  2. Update primaryLogin to newDefaultContactMethod 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.

@chiItepin
Copy link

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.

function generateReferralURL(email) {
return `${CONST.REFERRAL_PROGRAM.LINK}/?thanks=${encodeURIComponent(email)}`;
}

And how is being called below with the primaryLogin, so it's assuming it's not empty:

<CopyTextToClipboard
text={translate('referralProgram.copyReferralLink')}
textStyles={[styles.colorMuted]}
urlToCopy={generateReferralURL(account.primaryLogin)}

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.

@melvin-bot melvin-bot bot added the Overdue label Dec 3, 2023
@conorpendergrast
Copy link
Contributor

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:

  1. In Safari or Chrome on Mac OSX, go to staging.new.expensify.com
  2. Create a new user account
  3. Tap Search
  4. Tap on the referral banner at the bottom
  5. Tap Copy referral link
  6. Paste the link

Is that correct?

@melvin-bot melvin-bot bot removed the Overdue label Dec 5, 2023
@conorpendergrast conorpendergrast added the Needs Reproduction Reproducible steps needed label Dec 5, 2023
@chiItepin
Copy link

chiItepin commented Dec 5, 2023

@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

@situchan
Copy link
Contributor

situchan commented Dec 5, 2023

Taking this over based on @ntdiary's request

@situchan
Copy link
Contributor

situchan commented Dec 5, 2023

@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

Copy link

melvin-bot bot commented Dec 5, 2023

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

@tienifr
Copy link
Contributor

tienifr commented Dec 6, 2023

There was similar #19366 in the past and we decided to #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

@situchan
Copy link
Contributor

situchan commented Dec 6, 2023

Maybe @roryabraham has better context

@tgolen
Copy link
Contributor

tgolen commented Dec 7, 2023

Oof, that's a long issue. It would be great if you had a TLDR @roryabraham.

@ntdiary ntdiary removed their assignment Dec 8, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 11, 2023
Copy link

melvin-bot bot commented Dec 11, 2023

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

@melvin-bot melvin-bot bot added the Weekly KSv2 label Feb 15, 2024
@tienifr
Copy link
Contributor

tienifr commented Feb 15, 2024

PR ready for review #36556.

@muttmuure muttmuure added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 21, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Mar 4, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@tienifr
Copy link
Contributor

tienifr commented Mar 4, 2024

@miljakljajic PR was deployed to production 5 days ago. Please add Hold for payment label.

Copy link

melvin-bot bot commented Mar 5, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

Copy link

melvin-bot bot commented Mar 7, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr 10 days overdue. Is anyone even seeing these? Hello?

@roryabraham
Copy link
Contributor

looks like the PR just wasn't linked correctly or our automation wasn't working. The PR has been on prod for 2 weeks with no regressions, so we can pay it out ASAP.

@roryabraham roryabraham added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Mar 11, 2024
@roryabraham
Copy link
Contributor

@miljakljajic should be $500 each to @tienifr (C) and @situchan (C+)

@melvin-bot melvin-bot bot added the Overdue label Mar 13, 2024
Copy link

melvin-bot bot commented Mar 14, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Mar 17, 2024
@tienifr
Copy link
Contributor

tienifr commented Mar 18, 2024

@miljakljajic should be $500 each to @tienifr (C) and @situchan (C+)

@miljakljajic Could you proceed with this? Thanks

Copy link

melvin-bot bot commented Mar 18, 2024

@miljakljajic, @roryabraham, @situchan, @tienifr Still overdue 6 days?! Let's take care of this!

@miljakljajic
Copy link
Contributor

Contracts have expired - sent a new one to @situchan - @tienifr , what is your upwork profile?

@melvin-bot melvin-bot bot removed the Overdue label Mar 19, 2024
@tienifr
Copy link
Contributor

tienifr commented Mar 19, 2024

@miljakljajic Here’s my UW profile: https://www.upwork.com/freelancers/~01991fd5e5c11ef3ba

Thanks!

@miljakljajic
Copy link
Contributor

Extended the contract to you too @tienifr

@tienifr
Copy link
Contributor

tienifr commented Mar 19, 2024

@miljakljajic I’ve accepted the offer 🙏

@miljakljajic
Copy link
Contributor

Both contracts have now been paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Development

No branches or pull requests