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 get data requests #745

Closed
wants to merge 2 commits into from
Closed

Fix get data requests #745

wants to merge 2 commits into from

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Mar 7, 2024

Description

The code was attempting to use a JSONDecoder to decode responses when the caller was asking for plain Data. The fix is to make a separate function for getting Data responses without any JSON decoding.

Related discussion: p1709846681819879/1709766129.730749-slack-C04PWEZSYFL

ℹ Please replace the above with a link to the issue this pull request addresses, as well as a summary of the implementation details.

Testing Details

Test a clean install using wordpress-mobile/WordPress-iOS#22792 and verify that a feature announcement is shown (called "Stats Refresh"). You must log in using your A8c account.


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

The code was attempting to use a JSONDecoder to decode responses when the caller was asking for plain `Data`.
@guarani guarani requested a review from crazytonyli March 7, 2024 21:58
@guarani
Copy link
Contributor Author

guarani commented Mar 7, 2024

👋 Hey @crazytonyli, it looks like the tests need to be updated since the perform() function now has a new overload) and I didn't get to that today. I can loop back later tonight or tomorrow to fix. Feel free to hold off reviewing, or just review the code changes and I can update the tests and merge tomorrow 🙇

@guarani
Copy link
Contributor Author

guarani commented Mar 8, 2024

Closed in favor of #746.

@guarani guarani closed this Mar 8, 2024
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.

1 participant