Skip to content

Use correct api params for NewDot>OldDot links#5466

Merged
Jag96 merged 3 commits into
mainfrom
marcaaron-fixNewToOldDotLinks
Sep 23, 2021
Merged

Use correct api params for NewDot>OldDot links#5466
Jag96 merged 3 commits into
mainfrom
marcaaron-fixNewToOldDotLinks

Conversation

@marcaaron

Copy link
Copy Markdown
Contributor

Details

Looks like we changed the GetShortLivedAuthToken command to just return shortLivedAuthToken and encryptedAuthToken but didn't consider the change for NewDot > OldDot links.

Fixed Issues

#5359 (comment)

Tests

To test this I just quickly applied this diff and then tapped on manage cards (I don't have this set up on dev but I think it will work fine). Without the other changes we get the undefined stuff.

diff --git a/src/pages/workspace/WorkspaceCardPage.js b/src/pages/workspace/WorkspaceCardPage.js
index 4eee5d7f9..386c5a66e 100644
--- a/src/pages/workspace/WorkspaceCardPage.js
+++ b/src/pages/workspace/WorkspaceCardPage.js
@@ -100,13 +100,13 @@ const WorkspaceCardPage = ({
     }

     const onPress = () => {
-        if (user.isFromPublicDomain) {
-            openSignedInLink(CONST.ADD_SECONDARY_LOGIN_URL);
-        } else if (user.isUsingExpensifyCard) {
+        // if (user.isFromPublicDomain) {
+        //     openSignedInLink(CONST.ADD_SECONDARY_LOGIN_URL);
+        // } else if (user.isUsingExpensifyCard) {
             openSignedInLink(CONST.MANAGE_CARDS_URL);
-        } else {
-            openBankSetupModal();
-        }
+        // } else {
+        //     openBankSetupModal();
+        // }
     };

QA Steps

Repeat steps in #5359 (comment)

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron requested a review from a team as a code owner September 23, 2021 19:23
@marcaaron marcaaron self-assigned this Sep 23, 2021
@github-actions

Copy link
Copy Markdown
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 chiragsalian and removed request for a team September 23, 2021 19:23
cead22
cead22 previously approved these changes Sep 23, 2021

@cead22 cead22 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reproduced without this fix, and couldn't reproduce with this fix

Jag96
Jag96 previously approved these changes Sep 23, 2021

@Jag96 Jag96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, good find!

Comment thread src/libs/actions/App.js Outdated
@marcaaron marcaaron dismissed stale reviews from Jag96 and cead22 via da910c3 September 23, 2021 21:04

@Jag96 Jag96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Jag96

Jag96 commented Sep 23, 2021

Copy link
Copy Markdown
Contributor

Skipping unnecessary e2e to get this to staging

@Jag96 Jag96 merged commit 420504f into main Sep 23, 2021
@Jag96 Jag96 deleted the marcaaron-fixNewToOldDotLinks branch September 23, 2021 21:11
github-actions Bot pushed a commit that referenced this pull request Sep 23, 2021
Use correct api params for NewDot>OldDot links

(cherry picked from commit 420504f)
@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by @Jag96 in version: 1.1.1-7 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.1-8 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @Jag96 in version: 1.1.1-9 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @timszot in version: 1.1.2-7 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 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