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

[Awaiting Payment 26th Sept] [CRITICAL] Update the free trial badge to display Start a free trial badge when we create a workspace for a user #48217

Closed
danielrvidal opened this issue Aug 28, 2024 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@danielrvidal
Copy link
Contributor

danielrvidal commented Aug 28, 2024

Proposal: Update the free trial badge to display Start a free trial badge when we create a workspace for a user

Problem: Users don't really have a sense of urgency when it comes to configuring and using their workspace on creating it. Users also do not know right now that their workspace is a paid feature unless (1) a free trial starts, which does not happen till a user creates an expense on the workspace, which triggers the 7-day free trial to start that includes a badge on Concierge and in the Settings - Subscription tab says Free Trial: 7 days left or 2) they navigate to Settings -Subscription and learn about paid plans.

Solution: Show a Start a free trial badge on Concierge DM and in Settings - Subscriptions tab (LHN) when the workspace is first created.

  1. We already show this badge on Settings - Subscriptions tab itself when the workspace is first created. Leave that as is.
  2. When a user takes a billable action on that workspace, we currently show a Free trial: X days left in both the Concierge DM and Settings - Subscription tab (LHN). Keep this also as is. I.e. update the Start a free trial to Free trial: X days left when billable action is taken, which officially starts the free trial clock.

When a workspace is created, and there is no billable activity yet
image

image

Send a notification but lag this notification by a full minute so it isn't sent between the onboarding tasks itself
image

Issue OwnerCurrent Issue Owner: @trjExpensify
@danielrvidal danielrvidal added External Added to denote the issue can be worked on by a contributor Daily KSv2 labels Aug 28, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 28, 2024
Copy link

melvin-bot bot commented Aug 28, 2024

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

@neonbhai
Copy link
Contributor

Proposal

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

Update the free trial badge to display Start a free trial badge when we create a workspace for a use

What is the root cause of that problem?

Feature Request

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

We will update the code here, here, and here to use a util function to get the correct string according to trial status (as we do here)

function getFreeTrialBadgeText() {
    if (CardSectionUtils.shouldShowPreTrialBillingBanner()) {
        return translate('subscription.billingBanner.preTrial.title');
    } 
    if (SubscriptionUtils.isUserOnFreeTrial()) {
        return translate('subscription.billingBanner.trialStarted.title', {numOfDays: SubscriptionUtils.calculateRemainingFreeTrialDays()});
    }
    if (SubscriptionUtils.hasUserFreeTrialEnded()) {
        return translate('subscription.billingBanner.trialEnded.title');
    }
}

@abzokhattab
Copy link
Contributor

abzokhattab commented Aug 28, 2024

Edited by proposal-police: This proposal was edited at 2024-08-28 21:19:42 UTC.

Proposal

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

Update the free trial badge to display Start a free trial badge when we create a workspace for a user

What is the root cause of that problem?

Users lack urgency in configuring and using their workspace after creation. They are unaware that their workspace is a paid feature until either (1) they create an expense, which triggers the 7-day free trial (displayed as a badge in Concierge and Settings - Subscription), or (2) they manually navigate to Settings - Subscription to learn about paid plans.

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

  • Create PolicyUtils.hasPaidPolicy Function :
function hasPaidPolicy(policies: OnyxCollection<Policy> | null, currentUserAccountID: number): boolean {
    return Object.values(policies ?? {}).some(
        (policy) => isPolicyOwner(policy, currentUserAccountID ?? -1) && isPaidGroupPolicy(policy)
    );
}

This checks if the user owns a paid policy, which is the same approach used in useSubscriptionPlan , which is used in the subscription tab inside InitialSettingsPage.

  • Create getFreeTrialText Function :
function getFreeTrialText(policies: OnyxCollection<Policy> | null): string | undefined {
    if (!PolicyUtils.hasPaidPolicy(policies, currentUserAccountID)) {
        return undefined;
    }

    if (CardSectionUtils.shouldShowPreTrialBillingBanner()) {
        return translateLocal('subscription.billingBanner.preTrial.title');
    }
    if (isUserOnFreeTrial()) {
        return translateLocal('subscription.billingBanner.trialStarted.title', {numOfDays: calculateRemainingFreeTrialDays()});
    }
    if (hasUserFreeTrialEnded()) {
        return translateLocal('subscription.billingBanner.trialEnded.title');
    }

    return undefined;
}

This function will return the correct trial status text based on the user’s subscription status.
We’re passing policies as an input here so that it can recalculate the text whenever there's a change in the policies. This way, we always show the most accurate info.

  • Integration :
    • in HeaderView.tsx and InitialSettingsPage.tsx : Call getFreeTrialText.
    • In OptionRowLHN : Move the free trial text logic to the parent component LHNOptionsList to ensure updates to the trial status are reflected, since OptionRowLHN uses useMemo on input props and would not re-render if the title changes so we have to pass it as a prop to rerender.

Test Branch

Alternatively

  • We could use privateSubscription instead of hasPaidPolicy, but I prefer the new function since it aligns with the approach used in useSubscriptionPlan , which is used in the subscription tab inside InitialSettingsPage.

@anmurali
Copy link

I think @neonbhai is on the right track. I updated the mockups for clarity in the issue description. @rojiphil @chiragsalian - can you both take a quick review?

@rojiphil
Copy link
Contributor

I updated the mockups for clarity in the issue description. @rojiphil @chiragsalian - can you both take a quick review?

Yeah. It makes sense to display pre trial status in LHN and header of the chat used for onboarding.
Looks like this can be done in FE without any need of BE support by depending on shouldShowPreTrialBillingBanner and the presence of any paid policies.
@abzokhattab proposal looks more complete to me as we need to show the pre trial status only if there are any paid policies.
@chiragsalian What do you think?

@chiragsalian
Copy link
Contributor

Correct yeah BE is not needed for this the FE has all the information and i imagine its the same condition as when we determine to show subscription tab and then showing start a free trial banner in subscription page.

Reviewing @abzokhattab proposal,

disable the automatic start of the free trial.

I don't follow this. Currently and after the changes to solve this issue free trial will not start when a workspace is created.
Just to be clear, the issue here is to show a banner "Start a free trial" when workspace is created, it wont actually start a free trial.

Create PolicyUtils.getNumberOfPolicies which returns the policies count.

This doesn't feel right or rather precise to me. Yeah like rojiphil mentioned we need to check paid policies. So a helper function like hasPaidPolicy makes more sense to me that would short circuit/return immediately when it finds the first paid policy would be better.

Infact i would suggest using whatever logic the subscription tab uses to show itself so that our logic stays consistent with it. And looking at that code its here which looks at if NVP_PRIVATE_SUBSCRIPTION exists. So yeah using that looks even simpler because that NVP is set only when a workspace is created.

Also @danielrvidal / @anmurali, about this part,

Show a Start a free trial badge on Concierge DM

I think that's incorrect and the "Start a free trial badge" should be on whichever chat the user was onboarded with. Since that will contain the onboarding message.
When the free trial begins for such users the free trial badge will be on the Expensify DM and not Concierge
image

So it makes sense to keep having the badge in the same location as the onboarding chat otherwise it would jump around which is ugly.

@anmurali
Copy link

@chiragsalian - we're removing the Expensify DM and streamlining onboarding tasks always to Concierge, hence that description

@chiragsalian
Copy link
Contributor

Ah okay then perfect 👍 lets use Concierge and not worry about onboarding chat reportID

@danielrvidal
Copy link
Contributor Author

Good clarification though!

@melvin-bot melvin-bot bot added the Overdue label Sep 1, 2024
@abzokhattab
Copy link
Contributor

abzokhattab commented Sep 1, 2024

Thanks all for the feedback ... I have updated the proposal and added a test branch

here is the result:

Screen.Recording.2024-09-01.at.5.36.34.AM.mov

cc @chiragsalian @rojiphil

@rojiphil
Copy link
Contributor

rojiphil commented Sep 2, 2024

Infact i would suggest using whatever logic the subscription tab uses to show itself so that our logic stays consistent with it. And looking at that code its here which looks at if NVP_PRIVATE_SUBSCRIPTION exists. So yeah using that looks even simpler because that NVP is set only when a workspace is created.

Yeah. While using NVP_PRIVATE_SUBSCRIPTION seems simpler to me too, we can also use useSubscriptionPlan as referred to in the updated proposal if we want to keep our logic strictly consistent with show/hide of Subscription option in LHN for Settings. But I do not see the need to define another utility function i.e. hasPaidPolicy as we can directly make use of useSubscriptionPlan. Otherwise, the primary solution in @abzokhattab updated proposal LGTM.
@chiragsalian What do you think?

@melvin-bot melvin-bot bot removed the Overdue label Sep 2, 2024
@abzokhattab
Copy link
Contributor

But I do not see the need to define another utility function i.e. hasPaidPolicy as we can directly make use of useSubscriptionPlan

we can also change the new hasPaidPolicy to getOwnerPaidPolices so that it returns the paid policies that are owned by the user ... then we call this function in the useSubscriptionPlan as well

@chiragsalian
Copy link
Contributor

But I do not see the need to define another utility function i.e. hasPaidPolicy as we can directly make use of useSubscriptionPlan

I agree

we can also change the new hasPaidPolicy to getOwnerPaidPolices so that it returns the paid policies that are owned by the user ... then we call this function in the useSubscriptionPlan as well

I don't follow this comment. useSubscriptionPlan already checks for paid policies that are owned by the user. If i have misunderstood you can you explain this with a snippet.

@chiragsalian chiragsalian self-assigned this Sep 3, 2024
@abzokhattab
Copy link
Contributor

abzokhattab commented Sep 3, 2024

I mean the following changes:

  1. Instead of creating a hasPaidPolicy function inside PolicyUtils, we can create a getOwnedPaidPolicies function as shown below:
function getOwnedPaidPolicies(policies: OnyxCollection<Policy> | null, currentUserAccountID: number): Policy[] {
    return Object.values(policies ?? {}).filter((policy): policy is Policy => isPolicyOwner(policy, currentUserAccountID) && isPaidGroupPolicy(policy));
}
  1. Next, replace this snippet in the useSubscriptionPlan with:
const ownerPolicies = useMemo(() => PolicyUtils.getOwnedPaidPolicies(policies, session?.accountID ?? -1), [policies, session?.accountID]);
  1. Finally, in the getFreeTrialText function that I proposed, replace !PolicyUtils.hasPaidPolicy with:
const ownedPaidPolicies = PolicyUtils.getOwnedPaidPolicies(policies, currentUserAccountID);
if (isEmptyObject(ownedPaidPolicies)) {

This approach ensures that the logic for checking whether the user has a paid policy is consistent across both useSubscriptionPlan and getFreeTrialText. but this is one way ..the other way is to use the NVP_PRIVATE_SUBSCRIPTION but i prefer the first one to stay consistent with the useSubscriptionPlan which is called inside the subscription tab in the settings page

@chiragsalian
Copy link
Contributor

Your suggestion works. But i guess my original question (and even roji's) is why bother with a new hasPaidPolicy/getOwnedPaidPolicies when we could only just reuse useSubscriptionPlan. But as i think about this more i prefer your getOwnedPaidPolicies instead of useSubscriptionPlan since it eliminates the extra overhead to iterate and check for any control policies. So yeah i like getOwnedPaidPolicies.

Let's go with that. The rest of your proposal LGTM as well. Feel free to create the PR @abzokhattab.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2024
@abzokhattab
Copy link
Contributor

useSubscriptionPlan since it eliminates the extra overhead to iterate and check for any control policies. So yeah i like getOwnedPaidPolicies

agree ... also i chose this approach because we cannot import a hook inside the util files .. so in this case we cannot import the useSubscriptionPlan inside the PolicyUtils

@mallenexpensify mallenexpensify added the NewFeature Something to build that is a new item. label Sep 3, 2024
Copy link

melvin-bot bot commented Sep 3, 2024

Triggered auto assignment to @trjExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Sep 3, 2024
@danielrvidal
Copy link
Contributor Author

Still working through the PR listed above. Constant progress is being made.

@rojiphil
Copy link
Contributor

@danielrvidal @anmurali While the OP of this issue has not explicitly stated to display Your trial has ended in LHN for Onboarding chat and Subscription option, the PR includes this implementation. In a way, this also adds a sense of urgency to become a paid customer but I just wanted to cross-check if this is fine.

48217-trialended-confirm.mp4

@trjExpensify
Copy link
Contributor

I don't mind that, but it wasn't in the scope of the original design nor solves our problem stated in the OP really, so I'm hesitant to add it willy nilly. CC: @dannymcclain

@dannymcclain
Copy link
Contributor

I don't mind that, but it wasn't in the scope of the original design nor solves our problem stated in the OP really, so I'm hesitant to add it willy nilly.

I feel the same. Would love to get @dubielzyk-expensify's thoughts about it!

@dubielzyk-expensify
Copy link
Contributor

Agree with Tom and Danny here

@trjExpensify
Copy link
Contributor

Kewwwl, let's leave that off then @rojiphil.

@rojiphil
Copy link
Contributor

let's leave that off then @rojiphil.

Yeah. It's better to leave it as that's the consensus.

@abzokhattab Since the implementation mentioned here is not required, let us remove this from the PR. Thanks.

@danielrvidal
Copy link
Contributor Author

Woo! Just tested it and it looks great!
image

@trjExpensify trjExpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Sep 23, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@trjExpensify trjExpensify changed the title [CRITICAL] Update the free trial badge to display Start a free trial badge when we create a workspace for a user [Awaiting Payment 26th Sept] [CRITICAL] Update the free trial badge to display Start a free trial badge when we create a workspace for a user Sep 23, 2024
@trjExpensify
Copy link
Contributor

Nicee! Updated the title to reflect the payment hold.

@trjExpensify trjExpensify added Daily KSv2 and removed Weekly KSv2 labels Sep 25, 2024
@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2024
@trjExpensify
Copy link
Contributor

Payment will be due tomorrow.

@trjExpensify
Copy link
Contributor

Can we get a regression test for this please, @rojiphil?

@rojiphil
Copy link
Contributor

Can we get a regression test for this please, @rojiphil?

@trjExpensify Sure, the regression test is as follows:.

  1. Sign in with a new account.
  2. Complete the onboarding process using Track and Budget Expenses
  3. In the onboarding Concierge report, validate that the LHN and the header do not display the free trial badge
  4. Click on Settings->Workspaces->New Workspace and create a new workspace.
  5. Verify that the LHN for Concierge report, report header for Concierge report, and the subscription tab inside the account settings display the Start a free trial message.
  6. Submit a new expense to the workspace.
  7. Verify that the LHN for Concierge report, report header for Concierge report, and the subscription tab inside the account settings display the Free trial: 7 days left! message.

@trjExpensify trjExpensify added the Bug Something is broken. Auto assigns a BugZero manager. label Sep 26, 2024
Copy link

melvin-bot bot commented Sep 26, 2024

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

@trjExpensify trjExpensify removed the Bug Something is broken. Auto assigns a BugZero manager. label Sep 26, 2024
@trjExpensify
Copy link
Contributor

trjExpensify commented Sep 26, 2024

Excellent, issue created!

Payments as follows:

Offers sent!

@abzokhattab
Copy link
Contributor

Thanks a-lot, Tom for moving this forward

As this is a critical issue and has required some time and effort exploring different approaches, would it be possible to increase the bounty to 500 like those critical issues(#40152 (comment) - #40209 (comment) and others) ?

Your consideration is greatly appreciated.

cc @chiragsalian

@chiragsalian
Copy link
Contributor

Payment increase to 500 sounds fair to me. I let tom make the final decision and handle the payment.

@trjExpensify
Copy link
Contributor

Amended, paid @abzokhattab.

@rojiphil
Copy link
Contributor

@trjExpensify Accepted offer. Thanks.

@trjExpensify
Copy link
Contributor

Paid, closing!

@github-project-automation github-project-automation bot moved this from In Progress to Done in [#whatsnext] #convert Sep 27, 2024
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 Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
Development

No branches or pull requests

10 participants