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

Remove root array correctly for jsonp API requests #18136

Merged
merged 4 commits into from Oct 12, 2021

Conversation

justinvelluppillai
Copy link
Contributor

Description:

Fixes #18133 regression

Review

@justinvelluppillai justinvelluppillai added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Oct 12, 2021
@justinvelluppillai justinvelluppillai added this to the 4.6.0 milestone Oct 12, 2021
@tsteur
Copy link
Member

tsteur commented Oct 12, 2021

@justinvelluppillai could you add a test for this one for JSONP to ensure we still get correct output for JSONP? Could add a test for the API method that the user reported in the issue (Overlay.getTranslations).

@justinvelluppillai
Copy link
Contributor Author

@tsteur I've made a few changes in agreement with @tomudding's recent comment, and also added tests where they seemed most sensible (not sure if that was exactly where you were meaning to add tests in your comment?)

@justinvelluppillai justinvelluppillai changed the title Don't try to remove JSON root array for jsonp API requests Remove root array correctly for jsonp API requests Oct 12, 2021
@tsteur
Copy link
Member

tsteur commented Oct 12, 2021

That's what I meant. Tested it also and works

@tsteur tsteur merged commit f1cba6a into 4.x-dev Oct 12, 2021
@tsteur tsteur deleted the issue/18133-jsonp-api-request branch October 12, 2021 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSONp API request (with callback) strips first and last character from result
2 participants