Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[HOLD for payment 2024-11-21] Can't clear the free trial expired message from Concierge #50192

Open m-natarajan opened 1 month ago

m-natarajan commented 1 month ago

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: Reproducible in staging?: needs reproduction Reproducible in production?: needs reproduction 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: @zsgreenwald Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727985469399619

Action Performed:

Precondition: Payment card is added to the account

  1. Login to an account which has free trial left
  2. observe the LHN has "free trial expired" message

    Expected Result:

    Free trial expired message should not appear if the free trial is left and payment card added to the account

    Actual Result:

    Unable discard the "free trial expired" message from the LHN

    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
    • [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/8cf7b4ac-31a2-4c8c-8fa0-0415417a6030

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021841979108635835758
  • Upwork Job ID: 1841979108635835758
  • Last Price Increase: 2024-10-03
Issue OwnerCurrent Issue Owner: @mallenexpensify
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @mallenexpensify (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.

MelvinBot commented 1 month ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

abzokhattab commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-10-03 22:19:26 UTC.

Proposal

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

Can't clear the free trial expired message from Concierge

What is the root cause of that problem?

the free trial expired action is shown if !SubscriptionUtils.doesUserHavePaymentCardAdded()

https://github.com/Expensify/App/blob/9ebd9718dd44c2025ff936f0e9658338a3a15542/src/pages/settings/Subscription/CardSection/BillingBanner/TrialEndedBillingBanner.tsx#L10-L20 https://github.com/Expensify/App/blob/bee4bece7990553b714a47989f40f13b312711c2/src/libs/SubscriptionUtils.ts#L427-L432

The issue seems to be that the ONYXKEYS.NVP_BILLING_FUND_ID key is not being updated by the backend, causing it to remain null.

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

  1. we can either fix that in the backend
  2. or we can use getCardForSubscriptionBilling instead of doesUserHavePaymentCardAdded, which uses the ONYXKEYS.FUND_LIST key instead
    if (SubscriptionUtils.getCardForSubscriptionBilling()) {
        return null;
    }
  3. this change should be done here as well where we should add the following condition if (ReportActionsUtils.isActionableAddPaymentCard(action) && shouldRenderAddPaymentCard() && !SubscriptionUtils.getCardForSubscriptionBilling())

    What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

mallenexpensify commented 1 month ago

Removed Help Wanted for a min. @thesahindia , can you review @abzokhattab 's proposal above? Mainly curious if you think this likely needs to be worked on on the backend. Thx

mkzie2 commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-10-03 23:58:27 UTC.

Proposal

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

Unable discard the "free trial expired" message from the LHN

What is the root cause of that problem?

https://github.com/Expensify/App/blob/637b607578141b77247fc22b43e524442c3d03b3/src/pages/home/report/ReportActionItem.tsx#L400-L412

As we can see, we don't have a logic to hide that message if there is a payment card.

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

https://github.com/Expensify/App/blob/637b607578141b77247fc22b43e524442c3d03b3/src/pages/home/report/ReportActionItem.tsx#L400

can be:

        if (ReportActionsUtils.isActionableAddPaymentCard(action) && shouldRenderAddPaymentCard() && !hasPaymentCard) {
  1. SubscriptionUtils.doesUserHavePaymentCardAdded()

  2. SubscriptionUtils.getCardForSubscriptionBilling()

There two functions should be fix from BE side as well (if needed) to make sure it works properly. Now, as I can see, !!SubscriptionUtils.doesUserHavePaymentCardAdded() is always false.

image

What alternative solutions did you explore? (Optional)

mkzie2 commented 1 month ago

@mallenexpensify I think before user adds a payment card, the "Add payment card" button is displayed:

image

But after they add any payment card, we just need to hide that button instead of hiding all the message:

image

What do you think?

mkzie2 commented 1 month ago

Proposal updated

abzokhattab commented 1 month ago

Proposal Updated

thesahindia commented 1 month ago

Removed Help Wanted for a min. @thesahindia , can you review @abzokhattab 's proposal above? Mainly curious if you think this likely needs to be worked on on the backend. Thx

We can fix it solely on the front end, but backend changes would be ideal.

I think we should get an engineer here.

🎀 👀 🎀 C+ reviewed ( using it for engineer assignment )

melvin-bot[bot] commented 1 month ago

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

thesahindia commented 1 month ago

Quoting from https://github.com/Expensify/App/issues/50192#issuecomment-2392435590

https://github.com/Expensify/App/blob/bee4bece7990553b714a47989f40f13b312711c2/src/libs/SubscriptionUtils.ts#L427-L432

The issue seems to be that the ONYXKEYS.NVP_BILLING_FUND_ID key is not being updated by the backend, causing it to remain null.

cc: @youssef-lr

mkzie2 commented 1 month ago

@thesahindia The doesUserHavePaymentCardAdded is not related to checking whether we should display the free trial expired message. The issue is that, BE always returns the free trial expired message.

mkzie2 commented 1 month ago

@youssef-lr Also, please help check my comment when you have a chance. TY

melvin-bot[bot] commented 1 month ago

@youssef-lr, @mallenexpensify, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 1 month ago

@youssef-lr, @mallenexpensify, @thesahindia Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 1 month ago

@youssef-lr, @mallenexpensify, @thesahindia 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

mallenexpensify commented 1 month ago

@youssef-lr 👀 above plz. We need to know if you recommend fixing this on the back or frontend.

youssef-lr commented 1 month ago

I think this is better fixed in the backend @mallenexpensify

mkzie2 commented 1 month ago

I think this is better fixed in the backend @mallenexpensify

@youssef-lr What do you plan to fix in BE? Based on the RCA here, BE will not return the below message if there is a payment card, right?

image

mallenexpensify commented 1 month ago

@maddylewis added to #retain because "Being reported by a paying customer using NewDot, it should go in the #retain project" from here.

@abzokhattab and @mkzie2 , it's possible that we still might need help on the front end and/or that details you submitted here are used in the fix. If so, compensation might be due. We'd address that once the PRs been on production for a week.

mkzie2 commented 1 month ago

@mallenexpensify I just want to point out that the RCAs in this proposal and this one differ, with only one being accurate. Identifying the correct RCA is crucial to ensure we address the necessary fixes on the BE side. Otherwise, following the incorrect RCA could waste time without resolving the issue.

youssef-lr commented 1 month ago

I can take a look later today and report back here.

zsgreenwald commented 1 month ago

What's blocked us at the moment from fixing this issue? Can someone catch me up to speed?

melvin-bot[bot] commented 1 month ago

@youssef-lr @mallenexpensify @thesahindia this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

youssef-lr commented 1 month ago

Discussion here https://expensify.slack.com/archives/C07HPDRELLD/p1729185534722229

youssef-lr commented 1 month ago

We have landed on a different approach where we'll be adding a new message confirming that they added a card, and remove the action button from the previous message. I'll keep this one internal as I'll need to test optimistic data along with backend changes. Thanks everyone!

melvin-bot[bot] commented 4 weeks ago

@youssef-lr, @mallenexpensify, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

youssef-lr commented 3 weeks ago

Working on the PRs this week

melvin-bot[bot] commented 3 weeks ago

@youssef-lr, @mallenexpensify, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

youssef-lr commented 3 weeks ago

Made some progress in the PRs, I should have them ready by EOD.

thesahindia commented 2 weeks ago

@youssef-lr, let me know if you need help with the testing.

youssef-lr commented 2 weeks ago

Thanks! Will do once the backend is deployed.

youssef-lr commented 2 weeks ago

@thesahindia PR is ready for review

thesahindia commented 1 week ago

Thanks for letting me know. I will review it in the morning.

thesahindia commented 1 week ago

I created a new account and canceled the free trial from the old Dot but didn’t receive any message from the concierge—maybe because it’s different from the trial ending.

@youssef-lr, is it possible for you to end the free trial for my account (thesahindia+t14@gmail.com)?

I’m out right now; I’ll look into it more tomorrow.

mallenexpensify commented 1 week ago

ahhh.. sweet, I was able to end it by extending it to 2024-11-05 (vs the 12th)

Trial End Date: 2024-11-05 23:59:59 (expired)

thesahindia commented 1 week ago

I still haven’t received any messages from Concierge or Expensify. I also noticed that I should have received a message when my free trial had started. Is there anything specific I need to do to start receiving messages from Concierge and Expensify?

https://github.com/user-attachments/assets/1236aaf9-3c86-451d-937e-ec45c025f8fd

@mkzie2, @abzokhattab , could you guys help me with this if you have any information on it?

youssef-lr commented 1 week ago

@thesahindia try to submit an expense in the workspace, this should trigger the job that sends that message.

thesahindia commented 1 week ago

I had to select 'Manage my team’s expenses' during setup when signing up with a new account. This way I was able to get the automated message from Concierge confirming the start of the free trial. I think now I will be able to get the 'free trial expired' message once the trial ends.

Screenshot 2024-11-07 at 12 08 21 AM

Please end the free trial for thesahindia+t15@gmail.com

youssef-lr commented 1 week ago

Done @thesahindia, I recommend you submit an expense to trigger that message

thesahindia commented 1 week ago

Done @thesahindia, I recommend you submit an expense to trigger that message

Submitting an expense did trigger the message when I first signed up. Now I have to pay an expense to trigger concierge to send a message but I'm not sure why it's not sending me the 'free trial ended' message. I am getting a different message instead. Screen recording below:

https://github.com/user-attachments/assets/136e2f12-52a5-4c8a-a724-0a64d18c2f4c

youssef-lr commented 1 week ago

Can you try requesting money again and I'll check the logs?

thesahindia commented 1 week ago

Just did it.

youssef-lr commented 1 week ago

I just ran the job manually, you should receive the message now.

thesahindia commented 1 week ago

Thanks! Received it now.

thesahindia commented 1 week ago

@youssef-lr, Do we have any dummy cards available for testing? Couldn't find something on Slack.

mallenexpensify commented 1 week ago

@thesahindia the deets in the C+ doc for cards won't help ya, right? If you find a workaroud for your usecase and others C+ can use, can you add them to the C+ doc? thx

I'm back from OOO on Nov 14th, not assigning another BZ because payment won't be due til I'm back. If one is needed please add or post in #contributor-plus to ask for one to be added, thx.

youssef-lr commented 1 week ago

@thesahindia I'm writing from mobile, please google Stripe test cards.