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

Final Network Cleanup #8328

Merged
merged 14 commits into from
Mar 29, 2022
Merged

Final Network Cleanup #8328

merged 14 commits into from
Mar 29, 2022

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Mar 25, 2022

Details

Final Network cleanup PR. This one has less changes than the other two.

Summary of Changes

  • Moved "pre-request" and "error" handlers out of API.js and killed their related events
  • Centralized response handling logic in processRequest() instead of duplicated logic in 2 places h/t to @marcochavezf for this idea
  • Remove unused setSessionLoadingAndError() method as it does not appear to be needed and was setting an untranslated string.

Fixed Issues

No Issue - Related to Offline Doc

Tests

Existing automated tests should cover the changes made (we are just moving some logic around and reorganizing things).

QA

Make sure that offline report comments feature is still working as intended.

@marcaaron marcaaron self-assigned this Mar 25, 2022
Base automatically changed from marcaaron-networkCleanup2 to main March 28, 2022 18:26
@marcaaron marcaaron changed the title [WIP] Network Cleanup 3 Final Network Cleanup Mar 28, 2022
@marcaaron marcaaron marked this pull request as ready for review March 28, 2022 20:17
@marcaaron marcaaron requested a review from a team as a code owner March 28, 2022 20:17
@melvin-bot melvin-bot bot requested review from yuwenmemon and removed request for a team March 28, 2022 20:18
@marcaaron
Copy link
Contributor Author

cc @roryabraham @marcochavezf since you all reviewed the first 2 clean up PRs

@roryabraham roryabraham merged commit cb1925c into main Mar 29, 2022
@roryabraham roryabraham deleted the marcaaron-networkCleanup3 branch March 29, 2022 06:12
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.47-0 🚀

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

@roryabraham
Copy link
Contributor

@mvtglobally Can you please confirm that these QA steps are covered by existing regression tests? More detailed test steps should look something like this:

  1. Open a chat report.
  2. Put your device on airplane mode and disconnect from wifi.
  3. Send a few messages in the chat.
  4. Close the app.
  5. Reopen the app, verify you can still see the chats you sent while offline.
  6. Take your device off airplane mode and/or reconnect to wifi.
  7. Verify that you can still see the chats you sent while offline.
  8. Sign out and sign back in.
  9. Verify that you can still see the chats you sent while offline.

@OSBotify
Copy link
Contributor

OSBotify commented Apr 5, 2022

🚀 Deployed to production by @roryabraham in version: 1.1.49-1 🚀

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