[$500] Allow Desktop AhDoc and Dev builds to be installed alongside production

Julesssss commented 1 year ago

Problem

We recently enabled the ability to install mobile apps side by side, preventing the need to constantly reinstall and re-authenticate after testing a dev or AdHoc build. However, we didn't do the same thing for Desktop.

Solution

Let's do the same thing for Desktop!

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01af6dab8731944ef3
Upwork Job ID: 1690005246967926784
Last Price Increase: 2023-12-21

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

allroundexperts commented 1 year ago

Proposal

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

Build desktop app separately for different environments

What is the root cause of that problem?

We are using the same app id as defined here.

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

We need to use different appId for different environments. We can replace this with:

    appId: appIds[process.env.ELECTRON_ENV],

appIds can be defined as:

const appIds = {
    production: 'com.expensifyreactnative.chat',
    staging: 'com.expensifyreactnative.staging.chat',
    adhoc: 'com.expensifyreactnative.adhoc.chat',
};

We can do a similar thing for productName / dmg object and the name inside package.json if required. The logos seem to be correct for each environment right now except on About page which we can modify as well using the env variable. I think this can be handled in the PR!

What alternative solutions did you explore? (Optional)

None

Talha345 commented 1 year ago

Proposal

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

We want to allow multiple builds for each environment to be installed concurrently on a system.

What is the root cause of that problem?

This was never implemented which can bee seen in the config file as we have static values for appID, productName, and dmg etc.

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

We can make the configuration file dynamic and generate separate builds for each environment by making the following changes:

const appIds = {
    production: 'com.expensifyreactnative.chat',
    staging: 'com.expensifyreactnative.chat.staging',
    adhoc: 'com.expensifyreactnative.chat.adhoc',
};

Then we can use: appId: appIds[process.env.ELECTRON_ENV].

Similarly, we should follow the same pattern and update productName for clarity like:

productNames: {
    production: 'New Expensify',
    staging: 'New Expensify Staging',
    adhoc: 'New Expensify AdHoc',
},

and use it like productName: productNames[process.env.ELECTRON_ENV].

Same hash can be used for the dmg section and the following configs can be updated under dmg:

title: productNames[process.env.ELECTRON_ENV]
artifactName: `NewExpensify-${process.env.ELECTRON_ENV}-${version}.dmg`,

What alternative solutions did you explore? (Optional)

jeet-dhandha commented 1 year ago

Proposal

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

Build desktop app separately for different environments

What is the root cause of that problem?

Dynamic Values are not used for below details (Currently only used for s3Bucket):
- productName
- appId
- dmg.title
- dmg.artifactName
- protocols.name (Optional - Only if needed.)

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

We can use process.env.ELECTRON_ENV to set the values dynamically. (As suggested by everyone, i am just giving extra details, that's all.)


const env = process.env.ELECTRON_ENV;

// Below can be used to set values for productName, dmg.title
const names = {
    production: "New Expensify",
    staging: "New Expensify Staging",
    dev: "New Expensify Dev",
    adhoc: "New Expensify Adhoc",
};

const artifactName = names[env].split(" ").join("") + ".dmg";

const appIds = {
    production: "com.expensifyreactnative.chat",
    staging: "com.expensifyreactnative.staging.chat",
    dev: "com.expensifyreactnative.dev.chat",
    adhoc: "com.expensifyreactnative.adhoc.chat",
};

For logos we can use ExpensifyCashLogo.js to render logos dynamically. This was created for that sole purpose but haven't been used yet. (I don't know why.)
Also we will need to update color in new-expensify-dev.svg from .st2{fill:#002e22} to .st2{fill:#03D47C}. Others like new-expensify-staging.svg and new-expensify.svg are already using above color.
For other places like Share QR code, etc. can be updated using ExpensifyCashLogo.js.

What alternative solutions did you explore? (Optional)

N/A

Julesssss commented 1 year ago

Thanks all. Given the relative simplicity of these changes, I think going with the first proposal is fine.

melvin-bot[bot] commented 1 year ago

📣 @allroundexperts Please request via NewDot manual requests for the Contributor role ($1000)

allroundexperts commented 1 year ago

PR created.

melvin-bot[bot] commented 1 year ago

This issue has not been updated in over 15 days. @Julesssss, @ArekChr, @allroundexperts, @lschurr eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

Julesssss commented 1 year ago

We had some issues with the test builds, I just rebuilt and it should work this time.

Julesssss commented 1 year ago

This isn't a regression, as it was up to me to verify this... but the prod builds are failing due to these changes.

We're defaulting to the Dev environment when building the prod app...

 • signing         file=desktop-build/mac/New Expensify Dev.app identityName=Developer ID Application: Expensify, Inc.

Julesssss commented 1 year ago

Note: When running npm run build-desktop-adhoc locally, the build generated uses word marks of dev environment. This is because .env.adhoc file is missing on localhost.

@allroundexperts I think this happened, like you predicted.

melvin-bot[bot] commented 11 months ago

This issue has not been updated in over 15 days. @Julesssss, @ArekChr, @allroundexperts, @lschurr eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

allroundexperts commented 10 months ago

@Julesssss @lschurr What's the update here? The PR got merged long time ago.

Julesssss commented 9 months ago

Sorry, just saw this.

I think we should just close the issue as it's unlikely we'll find time to work on this

@allroundexperts do we need a 50% payment to make this fair? We merged a change but had to revert

allroundexperts commented 9 months ago

Yea, sounds good!

Julesssss commented 9 months ago

Hey @lschurr would you mind paying out $500 to @allroundexperts please when you have a moment. Thanks

melvin-bot[bot] commented 9 months ago

Upwork job price has been updated to $500

allroundexperts commented 9 months ago

I get paid through the App @Julesssss. A payment summary comment is all I need 😄

lschurr commented 9 months ago

Payment summary:

Contributor+: @allroundexperts $500

lschurr commented 9 months ago

I think we can close yeah?

JmillsExpensify commented 8 months ago

$500 payment to @allroundexperts based on comment above.

Expensify / App