Skip to content

Fix "Body should be a JSON Array" issue #48

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

Merged
merged 1 commit into from
Sep 13, 2013

Conversation

InventisPMTest
Copy link

The pull request http://github.com/KnpLabs/php-github-api/pull/40 fixed some issues with empty parameters, but caused a regression on other functionalities (see URL for a more detailed explanation).

This modification only forces JSON-objects to be generated when the parameters are empty. This should fix the newly created issues as well as not regress the fix for the previous issue.

@pilotmoon
Copy link

This fix is working for me too. I came here because of the regression to issue creation.

@shanem
Copy link

shanem commented May 16, 2013

Fix works for me, too. Please merge.

@sagotsky
Copy link

Me too. Is this in a tag yet?

@gquemener
Copy link
Contributor

Might be a silly question, but how could it be relevant to test $parameters emptiness when we already test that its size is > 0 the line before?

@@ -168,7 +168,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET',
$request = $this->createRequest($httpMethod, $path);
$request->addHeaders($headers);
if (count($parameters) > 0) {
$request->setContent(json_encode($parameters, JSON_FORCE_OBJECT));
$request->setContent(json_encode($parameters, empty($parameters)? JSON_FORCE_OBJECT : 0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add space before "?" please empty($parameters)? JSON_FORCE_OBJECT

@pilot
Copy link
Contributor

pilot commented Sep 1, 2013

@gquemener it's correct here, just wait @InventisPMTest to apply CS fix before merge

pilot added a commit that referenced this pull request Sep 13, 2013
Fix "Body should be a JSON Array" issue
@pilot pilot merged commit f0fc4e0 into KnpLabs:master Sep 13, 2013
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.

6 participants