AC-2144: PATCH method name support - #132
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the HTTP PATCH method in the code generator. It updates the MethodNameBuilder to return 'patch' for PATCH requests instead of falling through to 'update', and updates the corresponding RAML fixtures and expected generated client code to include a patchCategory method. There are no review comments, and the changes look correct.
e573a23 to
21a10b7
Compare
21a10b7 to
8ad7b3c
Compare
dzmitrybitsiutski
left a comment
dzmitrybitsiutski
left a comment
There was a problem hiding this comment.
Core change looks correct and well-scoped — the PATCH/PUT name collision is properly addressed and the category fixture exercises both methods on the same resource. A few inline notes below.
| public function __construct() | ||
| { | ||
| $this->arrayItemPopulatorCode = null; | ||
| $this->arrayItemConverterCode = null; | ||
| } |
There was a problem hiding this comment.
This constructor is a no-op and unrelated to PATCH name support. $arrayItemPopulatorCode / $arrayItemConverterCode are untyped properties, so they already default to null — assigning null in the constructor changes nothing. It also looks like scope creep for this PR. Suggest removing it, or, if it was added to silence a PHPStan/static-analysis warning, please note that in the PR description.
There was a problem hiding this comment.
True, it's a leftover from the previous MR that Marius asked to add
#130 (comment)
|
|
||
| ## 11.11.6 | ||
| ### Fixed | ||
| - PHP code generator now generates a distinct `patch` method name for PATCH endpoints instead of reusing the `update` name from PUT, avoiding collisions when both PATCH and PUT are defined on the same resource |
There was a problem hiding this comment.
Worth flagging the backward-compatibility impact: this also renames generated methods for resources that have a PATCH-only endpoint (update* -> patch*), which breaks downstream code calling the old name after regeneration. Released as a patch bump under Fixed — consider calling this out explicitly as a potential BC break (or a minor bump).
There was a problem hiding this comment.
True, updated the description and version
| * @param Entities\Category $category | ||
| * @return Entities\Category | ||
| */ | ||
| public function patchCategory($id, Entities\Category $category) |
There was a problem hiding this comment.
Check https://github.com/paysera/rest-style-guide#http-methods - we don't even use PATCH, PUT should be used to updated resource.
There was a problem hiding this comment.
Having separate method for PATCH doesn't hurt. Currently app-accounting-api uses PATCH method, and we cannot generate client from api-spec for it.
3 participants
No description provided.