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 IOU currency routes #3932

Merged
merged 2 commits into from
Jul 8, 2021
Merged

Fix IOU currency routes #3932

merged 2 commits into from
Jul 8, 2021

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jul 8, 2021

Details

Just some broken routes – we were trying to go to /iou/split//currency, where the missing section is meant to be a reportID. However, in this case it's perfectly legitimate to not have a reportID, because we're creating a new bill split. So instead, we can fix this by going to /iou/split/:reportID/currency. It actually doesn't matter what's between split/ and /currency, as long as it's not empty. Just x seems to work as well.

Fixed Issues

$ #3920

Tests / QA Steps

  1. Click on the big green FAB.
  2. Select Split Bill
  3. Click on the currency symbol in the "Split Bill" modal.
  4. Verify that the currency selection page opens up.
  5. Close the modal.
  6. Click on the big green FAB.
  7. Select Request Money
  8. Click on the currency symbol in the "Request Money" modal.
  9. Verify that the currency selection page opens up.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

CurrencyRouteWeb.mov

Mobile Web

CurrencyRouteMWeb.mov

Desktop

CurrencyModalDesktop.mov

iOS

IOUCurrencyIOS.mov

Android

@roryabraham roryabraham self-assigned this Jul 8, 2021
@roryabraham roryabraham requested a review from a team as a code owner July 8, 2021 16:24
@MelvinBot MelvinBot requested review from HorusGoul and removed request for a team July 8, 2021 16:24
@roryabraham roryabraham requested a review from cead22 July 8, 2021 16:47
@OSBotify
Copy link
Contributor

OSBotify commented Jul 8, 2021

⚠️ ⚠️ 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.

@cead22 cead22 merged commit 1cc22a1 into main Jul 8, 2021
@cead22 cead22 deleted the Rory-FixIOUCurrencyRoutes branch July 8, 2021 18:05
github-actions bot pushed a commit that referenced this pull request Jul 8, 2021
Fix IOU currency routes

(cherry picked from commit 1cc22a1)
@roryabraham
Copy link
Contributor Author

This PR was CP'd in 1.0.76-1. The normal deploy comment wasn't posted here, seemingly due to a random network error.

@isagoico
Copy link

isagoico commented Jul 9, 2021

@roryabraham Should go ahead and test this PR?

@roryabraham
Copy link
Contributor Author

Yep, it's on staging. Go ahead and test it please 🙇

@roryabraham
Copy link
Contributor Author

Actually @isagoico I already tested this and closed the linked issue. I'm going to check this off the deploy checklist.

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.77-5🚀

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.

4 participants