Skip to content

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

Merged
dunglas merged 1 commit into
api-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 into
api-platform:2.0from
soyuka:fix/1122

Conversation

@soyuka

@soyuka soyuka commented May 17, 2017

Copy link
Copy Markdown
Member

…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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

dunglas commented May 21, 2017

Copy link
Copy Markdown
Member

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

@soyuka

soyuka commented May 22, 2017

Copy link
Copy Markdown
Member Author

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

@soyuka

soyuka commented May 22, 2017

Copy link
Copy Markdown
Member Author

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

dunglas commented May 22, 2017

Copy link
Copy Markdown
Member

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants