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: remove the replacement of escape characters in json response formatting #3163

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

musale
Copy link
Contributor

@musale musale commented May 29, 2024

Overview

Removes replacement of escaped characters in the stringified json.
closes #3158

Testing Instructions

  • run the query https://graph.microsoft.com/v1.0/appCatalogs/teamsApps?$expand=*
  • Check the response preview tab to have well formatted json

Signed-off-by: Musale Martin <martinmusale@microsoft.com>
@musale musale requested a review from a team as a code owner May 29, 2024 16:23
@musale musale changed the title fix: remove character escape in json response formatting fix: remove the replacement of escape characters in json response formatting May 29, 2024
Copy link
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3163.centralus.azurestaticapps.net

@thewahome
Copy link
Collaborator

@musale have you confirmed this to work with Safari?
Though done a long time back, the replacement code was supposed to fix this issue #895 .

If this change makes the replacement code unnecessary, then we might not need to stringify then parse. We can just pass the Json as is and get rid of this function

@musale
Copy link
Contributor Author

musale commented May 31, 2024

Pinging @Onokaev to validate this on Safari.

@SilasKenneth
Copy link
Member

SilasKenneth commented Jun 3, 2024

Tested in Safari on iOS 17.5.1

———

IMG_0444

@musale
Copy link
Contributor Author

musale commented Jun 3, 2024

Thank you @SilasKenneth! Also tested the paths in the #895 and they work as expected.

@thewahome
Copy link
Collaborator

@musale since the test works, the stringify -> parse is an unnecessary set of legacy steps. Is this something that we can confidently remove?

@musale
Copy link
Contributor Author

musale commented Jun 3, 2024

@musale since the test works, the stringify -> parse is an unnecessary set of legacy steps. Is this something that we can confidently remove?

Sure! Unless that telemetry is important, I don't see the need for that function too.

Copy link
Contributor

github-actions bot commented Jun 7, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3163.centralus.azurestaticapps.net

Copy link
Contributor

github-actions bot commented Jun 7, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://jolly-sand-0ac78c710-3163.centralus.azurestaticapps.net

@thewahome thewahome merged commit 7c4ee1d into dev Jun 7, 2024
17 of 20 checks passed
Copy link

sonarcloud bot commented Jun 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@thewahome thewahome mentioned this pull request Jun 18, 2024
thewahome added a commit that referenced this pull request Jun 18, 2024
* refactor: use strong types in authentication module (#3164)

* Chore: Dependabot upgrades - April (#3124)

* fix: remove character escape in json response formatting (#3163)

* Fix: add parameters to token request (#3218)

* Bump version to 10.2.0
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.

Graph Explorer does not return Json formatted output
3 participants