Skip to content
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

[API] Add dicom upload #9154

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MaximeMulder
Copy link
Contributor

Brief summary of changes

This is a port of #6161, which contains a description of the changes. Since the old PR has diverged quite a bit from the main branch, and that its author no longer works here, I found it easier to copy the changes on the current main branch and open a new PR rather than to rebase the PR. While porting, I only rewrote the old PR code whenever static analysis or practical stests showed that it was no longer in sync with the main branch (or when my IDE removed trailing spaces).

Testing instructions (if applicable)

  • Test the four new API routes, described in section 5 of the documentation of the API v0.0.4.

I must say I am not exactly sure how the API will be used, so I just made sure behaviour matches the documentation. I am still not sure whether $request->getParsedBody() or json_decode($request->getBody()->__toString(), true) should be used to get the request parameters since they seem to return different result.

@MaximeMulder
Copy link
Contributor Author

MaximeMulder commented Mar 20, 2024

I must say I am not sure what I am supposed to do regarding the failed tests on files that I did not modify in the PR. Any direction @driusan (or someone else) 🤔.

@kongtiaowang
Copy link
Contributor

kongtiaowang commented Apr 24, 2024

Line php/libraries/FilesDownloadHandler.php
71 Parameter #1 $value of function strval expects
bool|float|int|resource|string|null, mixed given.

Line php/libraries/SiteIDGenerator.php
81 Negated boolean expression is always false.
💡 Because the type is coming from a PHPDoc, you can turn off this
check by setting treatPhpDocTypesAsCertain: false in your
./test/phpstan-loris.neon.
297 Parameter #1 $value of function strval expects
bool|float|int|resource|string|null, mixed given.
377 Parameter #1 $value of function strval expects
bool|float|int|resource|string|null, array<int,
int|string>|string|null given.
387 Parameter #1 $value of function strval expects
bool|float|int|resource|string|null, array<int, int|string>|string
given.
400 Parameter #1 $value of function strval expects
bool|float|int|resource|string|null, array<int, int|string>|string
given.

@MaximeMulder
Copy link
Contributor Author

Hum, yeah, I already saw that.

I was just suprised to see that the errors are marked as originating from FilesDownloadHandler.php and SiteIDGenerator.php, which are not modified by this PR. I guess I must call these files from elsewhere and that the location reporting is just imprecise.

I'll tackle this tomorrow probably.

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.

None yet

2 participants