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

[HOLD for payment 2021-12-06] Add app download links to workspace invitation validation email - Reported by: @akshayasalvi #5774

Closed
isagoico opened this issue Oct 12, 2021 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@isagoico
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Invite a new user account (that you have access to) to a workspace
  2. Check the validation email

Expected Result:

If the apps are mentioned in the email there should be a download link for iOS and Android.

Actual Result:

Apps are mentioned in email but no download link is included.

Workaround:

None needed.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.7-3

Reproducible in staging?: yes
Reproducible in production?: yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

Issue reported by: @akshayasalvi
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1633983814141300

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @madmax330 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@madmax330 madmax330 added the External Added to denote the issue can be worked on by a contributor label Oct 12, 2021
@MelvinBot
Copy link

Triggered auto assignment to @MitchExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@madmax330 madmax330 removed their assignment Oct 12, 2021
@akshayasalvi
Copy link
Contributor

akshayasalvi commented Oct 12, 2021

Proposed Solution

Would want to know the preferences, but if it is going to be plain simple links. We can update the links in the es.js and en.js

en: {
  welcomeNote: ({workspaceName}) => `You have been invited to ${workspaceName}! Download the Expensify mobile app to start tracking your expenses. <a href='${androidUrl}'>Android</a>, <a href='${iosUrl}'>iOS</a>, <a href='${desktopUrl}'>Desktop</a>`
}

In WorkspaceInvitePage,

getWelcomeNotePlaceholder() {
        return this.props.translate('workspace.invite.welcomeNote', {
            workspaceName: this.props.policy.name,
            androidUrl:  CONST.APP_DOWNLOAD_LINKS.ANDROID,
            iosUrl: CONST.APP_DOWNLOAD_LINKS.IOS,
            desktopUrl: CONST.APP_DOWNLOAD_LINKS.DESKTOP,
        });
    }

Guessing we change to modify the content a bit to make it more reader-friendly.

Can implement the same by passing an array of links instead of hardcoding the platforms in the string. (Might be an overkill unnecessarily.)

@MitchExpensify
Copy link
Contributor

Will get to this tomorrow, focusing on N6 right now

@MelvinBot MelvinBot removed the Overdue label Oct 15, 2021
@MelvinBot
Copy link

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

@MitchExpensify
Copy link
Contributor

Upwork job

@MelvinBot MelvinBot added Overdue and removed Overdue labels Oct 19, 2021
@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 Overdue labels Oct 22, 2021
@MelvinBot
Copy link

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

@Beamanator
Copy link
Contributor

Beamanator commented Oct 22, 2021

@akshayasalvi I like this idea & your implementation 👍 Please submit a PR when you have a chance (& apply to the upwork job)!

I don't think you'll need to pass the links in via this.props.translate('workspace.invite.welcomeNote', { unless we start having multiple custom welcome notes :) In en.js & es.js you can use the constants though 👍

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 22, 2021
@akshayasalvi
Copy link
Contributor

@Beamanator If I add the content to the message it shows up the raw HTML like this

image

I was wondering if I should update only the API call by appending the download app links.

  invite(logins, this.state.welcomeNote || this.getWelcomeNotePlaceholder(), this.props.route.params.policyID);
    ```

@Beamanator
Copy link
Contributor

Hmm good point @akshayasalvi - Personally I think it's fine, but what do you have in mind for just appending in the api Call?

@akshayasalvi
Copy link
Contributor

Yeah I was wondering if we could just append in the app call.

@Beamanator
Copy link
Contributor

I'm interested in your specific thoughts b/c I think we'd only be able to append directly to this.getWelcomeNotePlaceholder()... We can't just append to this.state.welcomeNote because it's possible the user deleted and rewrote their welcome note, so the download links would be weird there. But how do we know if the current value of this.state.welcomeNote is the default welcome note or something that the user specifically wrote?

@akshayasalvi
Copy link
Contributor

We're setting the default welcomeNote in state as this.getWelcomeNotePlaceholder(). and we're calling invite(logins, this.state.welcomeNote || this.getWelcomeNotePlaceholder(), this.props.route.params.policyID);.

because it's possible the user deleted and rewrote their welcome note, so the download links would be weird there
I get what you mean. Even we could maintain a flag, we can't be sure that the user removed the download app content or retained. Should we add some kind of a placeholder? So that if it exists, we'll append else we won't?

App links {{Android}}, {{iOS}} and {{Desktop}}

With a small note at the bottom of the field?

@Beamanator
Copy link
Contributor

App links {{Android}}, {{iOS}} and {{Desktop}}

With a small note at the bottom of the field?

Can you please explain what you're suggesting here? It sounds like you're saying we can add just App links {{Android}}, {{iOS}} and {{Desktop}} in the getWelcomeNotePlaceholder test, and before we send it using invite we can replace the {{Android}} with the Android download links, etc.?

@akshayasalvi
Copy link
Contributor

yeah that's what I am saying

@Beamanator
Copy link
Contributor

Hmm this could work well, but I'd like to hear what other people think. It's possible {{Android}}, {{iOS}} and {{Desktop}} makes sense for developers but won't make sense for normal users 🤔 Do you mind asking in #expensify-open-source what people prefer out of these options:

  1. Go with the original idea, which shows the links in raw html here
  2. Go with your latest suggestion here
  3. Something else?

@Beamanator
Copy link
Contributor

As mentioned here, we decided to link directly to this page which will have the download links: https://use.expensify.com/download

@Beamanator
Copy link
Contributor

Actually I'm going to post again to the main channel to get a few more 👍 / 👎 on the idea before moving forward with this idea

@akshayasalvi
Copy link
Contributor

PR Raised

@Beamanator
Copy link
Contributor

Not overdue, just waiting to get to production for payment 👍

@MelvinBot MelvinBot removed the Overdue label Nov 22, 2021
@akshayasalvi
Copy link
Contributor

@MitchExpensify Need your help on this one. It seems I wasn't formally hired on Upwork.I clicked on the Upwork link and it says the job is no longer available.

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 29, 2021
@botify botify changed the title Add app download links to workspace invitation validation email - Reported by: @akshayasalvi [HOLD for payment 2021-12-06] Add app download links to workspace invitation validation email - Reported by: @akshayasalvi Nov 29, 2021
@botify
Copy link

botify commented Nov 29, 2021

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.16-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-12-06. 🎊

@akshayasalvi
Copy link
Contributor

@MitchExpensify Need your help on this one. It seems I wasn't formally hired on Upwork.I clicked on the Upwork link and it says the job is no longer available.

@MitchExpensify Bump

@MitchExpensify
Copy link
Contributor

Sorry for the delay @akshayasalvi, the OG Upwork job expired. Please apply to the new one I've created here. Thanks!

@akshayasalvi
Copy link
Contributor

Thanks for the posting @MitchExpensify. I’ve applied.

@MitchExpensify
Copy link
Contributor

Hired @akshayasalvi ! Sorry for the delay, I was ooo

@akshayasalvi
Copy link
Contributor

Thanks @MitchExpensify. Awaiting payment now 😄

@MitchExpensify
Copy link
Contributor

Payment with bonus for reporting made @akshayasalvi

@Beamanator
Copy link
Contributor

I guess we can close now right @MitchExpensify ? :D

@MitchExpensify
Copy link
Contributor

Yes!

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 Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants