-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Share Code URL implementation using Environment URL #21107
Conversation
@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@mananjadhav Were you able to review and test the changes? Let me know if this is fine to merge. Or if any changes are required, will be glad to update. |
@alex-mechler. I hope you are back today. Can you please add [email protected] and [email protected] to the high traffic account? |
@mananjadhav, @alex-mechler, Friendly bump on review and next steps. |
Added [email protected] and [email protected] as high traffic accounts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good, I am done testing it on 3 platforms. I'll finish this in next 10 mins.
Reviewer Checklist
Screenshots/VideosWebweb-env-url.movMobile Web - Chromemweb-chrome-env-url.movMobile Web - Safarimweb-safari-env-url.movDesktopdesktop-env-url.moviOSios-env-url.movAndroidandroid-env-url.mov |
@alex-mechler all yours for review, I checked the URL by copying values from @rojiphil I can see your one checklist item missing related to high-traffic account. Your account has been added, so just test is once and then complete the checklist. Lastly I see one lint issue in |
@mananjadhav @alex-mechler The lint issue due to prettier has been fixed. Also, I have done testing with the high-traffic account and have updated the checklist too with all checks. Kindly review and approve. |
We did not find an internal engineer to review this PR, trying to assign a random engineer to #19464... Please reach out for help on Slack if no one gets assigned! |
@mananjadhav @MelvinBot AFAIK, @alex-mechler was supposed to review this as an internal engineer. Or is there any change? |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/flodnv in version: 1.3.31-0 🚀
|
🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.31-3 🚀
|
🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.31-3 🚀
|
@mananjadhav @alex-mechler @kevinksullivan @AndrewGable
Details
The following changes can be done to incorporate the functionality in a common place:
a) Leverage the existing
WithEnvironment
component and add support forenvironmentURL
. This variable will contain the environment-specific URL fromEnvironment
.b) Use the
environmentURL
fromWithEnvironment
to decide on the URL for use withShare code
functionality. Also, ensure that a trailing slash is present in the environment url.Fixed Issues
$ #19464
PROPOSAL: #19464 (comment)
Tests
Prerequisite:
Find out the latest production version at https://api.github.com/repos/expensify/app/releases/latest.
Tag_name will represent the latest deployed production version
"tag_name": "1.3.27-7",
Here, the latest deployed production version is 1.3.27-7
i) Staging:
Build an android app that uses a production environment configuration file with package version of 1.3.27-8
"version": "1.3.27-8", (i.e. in package.json)
Follow the steps as below:
a) Verify version in Settings->About
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://staging.new.expensify.com/
ii) Production:
Build an android app that uses production environment configuration file with package version of 1.3.27-7
"version": "1.3.27-7", (i.e. in package.json)
Follow the steps below:
a) Verify version in Settings->About
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
iii) Development:
Follow the steps as below:
a) Verify 'dev' label next to
Expensify
in header.b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://localhost:8080/
i) Development Version:
a) Launch the expensify app at http://localhost:8080/
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://localhost:8080/
i) Production Version:
a) Launch the electron production App
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
ii) Staging Version:
a) Launch the electron staging App
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://staging.new.expensify.com/
i) Production Environment:
a) Launch the production iOS Native app
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
i) MacOS/iOS-Safari
a) Launch the dev version on Safari in MacOS
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://localhost:8080/
Offline tests
i) Android-Native Production Version:
a) Ensure mobile is offline (Disconnect from WiFi and Mobile Data)
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
QA Steps
Following additional test steps are required:
i) Staging Deployment:
a) Launch the expensify app at https://staging.new.expensify.com/
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://staging.new.expensify.com/
ii) Production Deployment:
a) Launch the expensify app at https://new.expensify.com/
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
i) Staging:
a) Setup TestFlight for iOS and launch app
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://staging.new.expensify.com/
ii) Production:
a) Install production version from iOS App Store and launch app.
b) Copy URL to keyboard in Settings->Share code
c) Paste it in the compose box of one of the existing chat
d) Verify that the URL starts with https://new.expensify.com/
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android-Native-Staging (1->i)
v3_andr_nat_stag.mp4
Android-Native-Production (1->ii)
v3_andr_nat_prod.mp4
mWeb-Chrome-Development Version (2->i)
v3_mweb_dev.mp4
MacOS / Desktop Production
v3_dtop_prod.mp4
MacOS / Safari -Dev
v3_macos_safari.mp4
iOS / Native - Simulator - Production
new_iosnative_prod.mp4
MacOS / Desktop Staging
new_dtop_stag.mp4
Offline - Android Native Prod
new_offline_android_prod.mp4