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

fix(env): regression with prod env not being properly read #7714

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

tlebon
Copy link
Contributor

@tlebon tlebon commented Mar 27, 2024


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

fixes a regression introduced in #7106

Causes (Optional)

customWebappUrl is not an accurate way to determine if the app is productionEnv. Additionally, staging url has been deprecated.

electron/src/main.ts Outdated Show resolved Hide resolved
Comment on lines 54 to 57
const URL_WEBSITE = {
PRODUCTION: config.websiteUrl,
STAGING: 'https://wire-website-staging.zinfra.io',
STAGING: 'https://wire.com',
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can just use config.websiteUrl for all the case, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if thats the case, lets just remove the isProdEnvironment var

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd totally agree with that 👍 Let simplify this

Comment on lines 42 to 45
const isProdEnvironment =
!!customWebappUrl ||
customWebappUrl === webappEnvironments[ServerType.PRODUCTION].url ||
currentEnvironment === ServerType.PRODUCTION;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changing this condition? Not sure I'm following here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasnt working correctly before. thats where the regression came from. even in testing i found this condition sort of flakey sometimes (being true at startup but not later), thats why i included several cases.

@tlebon tlebon enabled auto-merge (squash) March 27, 2024 16:18
@tlebon tlebon merged commit b816d0b into dev Mar 27, 2024
5 checks passed
@tlebon tlebon deleted the fix/update-env-url branch March 27, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants