Skip to content

Comments

Swagger documentation normalizer revert throw when method is not supp…#1124

Merged
dunglas merged 1 commit intoapi-platform:2.0from
soyuka:fix/1122
May 22, 2017
Merged

Swagger documentation normalizer revert throw when method is not supp…#1124
dunglas merged 1 commit intoapi-platform:2.0from
soyuka:fix/1122

Conversation

@soyuka
Copy link
Member

@soyuka soyuka commented May 17, 2017

…orted

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1122
License MIT
Doc PR n/a

@@ -171,7 +171,6 @@ private function getPathOperation(string $operationName, array $operation, strin
case 'DELETE':
return $this->updateDeleteOperation($pathOperation, $resourceShortName);
default:
Copy link
Member

Choose a reason for hiding this comment

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

this line should be reverted and the return statement on the last line added back.

@@ -1111,6 +1111,8 @@ public function testNoOperations()
*/
public function testMethodNotSupported()
Copy link
Member

Choose a reason for hiding this comment

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

We may want to use custom methods (standard ones like PATCH, but also custom ones like BAN or PURGE).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I skipped the test, I'll add one with custom methods.

@soyuka soyuka force-pushed the fix/1122 branch 2 times, most recently from 422cdf8 to 6f11e6f Compare May 17, 2017 12:35

$expected = [
'swagger' => '2.0',
'basePath' => null,
Copy link
Member Author

Choose a reason for hiding this comment

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

ping @Simperfit is this correct? I think that this custom method behavior isn't correctly supported no?

@soyuka soyuka added the bug label May 17, 2017
@soyuka soyuka requested a review from Simperfit May 19, 2017 08:45
@dunglas
Copy link
Member

dunglas commented May 21, 2017

The patch looks good to me. Can you add the missing test?

@soyuka
Copy link
Member Author

soyuka commented May 22, 2017

I added the missing test already, but the result seems weird.

@soyuka
Copy link
Member Author

soyuka commented May 22, 2017

Okay, test is fine after second look. Rebased feel free to merge once green.

@dunglas dunglas merged commit fcc70fa into api-platform:2.0 May 22, 2017
@dunglas
Copy link
Member

dunglas commented May 22, 2017

Thanks @soyuka!

@soyuka soyuka deleted the fix/1122 branch May 22, 2017 08:46
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
Swagger documentation normalizer revert throw when method is not supp…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants