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

[No QA] Change expensify.cash => new.expensify.com #4016

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Conversation

rafecolton
Copy link
Member

Details

Fixed Issues

Part of https://github.com/Expensify/Expensify/issues/161422

Tests

N/A

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@rafecolton rafecolton requested a review from a team as a code owner July 13, 2021 21:20
@rafecolton rafecolton self-assigned this Jul 13, 2021
@botify
Copy link

botify commented Jul 13, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@OSBotify
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@MelvinBot MelvinBot requested review from Gonals and removed request for a team July 13, 2021 21:20
<action android:name="android.intent.action.VIEW"/>
<category android:name="android.intent.category.DEFAULT"/>
<category android:name="android.intent.category.BROWSABLE"/>
<data android:scheme="https" android:host="www.expensify.cash" />
Copy link
Contributor

Choose a reason for hiding this comment

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

We previously had a separate filter for expensify.cash and www.expensify.cash, shouldn't we add one here for www.new.expensify.com? Context for this in #2052 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea - @marcaaron?

@@ -8,6 +8,7 @@ export default {
'https://www.expensify.cash',
'https://staging.expensify.cash',
'http://localhost',
'https://new.expensify.com',
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here, do we also need https://www.new.expensify.com?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't planning on adding it, since I think it's less likely somebody would type in www.new, but that does remind me that I need to add staging.new

@roryabraham
Copy link
Contributor

If the actual domain name from which the site is hosted is changing, I think we'll also need to modify this and it's associated usages

@rafecolton
Copy link
Member Author

Yep good call @roryabraham, those do need to be updated as well

@roryabraham
Copy link
Contributor

roryabraham commented Jul 13, 2021

I think that one's just used here. IDK if you also want to change the App name or site title as well. Those are just visual changes I believe, so could be handled separately.

@roryabraham
Copy link
Contributor

Might also want to change this guy

@roryabraham
Copy link
Contributor

Also this and this are important if the cloudflare domain is being changed.

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

LGTM @roryabraham all yours

@roryabraham
Copy link
Contributor

I think this looks good now.

@rafecolton rafecolton changed the title [No QA][HOLD] Change expensify.cash => new.expensify.com [No QA] Change expensify.cash => new.expensify.com Jul 14, 2021
@botify
Copy link

botify commented Jul 14, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@rafecolton rafecolton merged commit 9d900a8 into main Jul 14, 2021
@rafecolton rafecolton deleted the new-expensify-com branch July 14, 2021 18:14
github-actions bot pushed a commit that referenced this pull request Jul 14, 2021
[No QA] Change expensify.cash => new.expensify.com

(cherry picked from commit 9d900a8)
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.77-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.77-5🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.77-6🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.79-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants