-
Notifications
You must be signed in to change notification settings - Fork 102
make server api compliant fixes #1464 #1477
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
Conversation
2eb20af to
48bb555
Compare
christianlupus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few issues with this PR:
- You should create an individual filter. See here and an example.
- No tests are provided. See above to simplify this.
- The behavior of the API is changing. This must be reflected in the API version.
- A corresponding entry to the changelog would be needed.
- Once, the PR is ready, the devs need to be warned about the upcoming change in the API.
|
PS: Have you checked this is not causing trouble with the Vue frontend? I have not checked the branch version. |
|
Sounds resonable :) I'll look into the filter stuff but can't promise my php is good enough for that. EDIT:
I haven't seen any problems but I haven't thoroughly tested it either. |
|
You can also close this PR or tell me. I would then create/update a new PR with an appropriate filtering method. |
|
If I had the time I would accept the challenge but it's probably better If you do it. I think you should be able to push to this PR? Or just close it if you want. |
|
I will try to fix it and push to the repository. I have not tried it yet. |
…lues Signed-off-by: Christian Wolf <[email protected]>
48bb555 to
e8d2926
Compare
Signed-off-by: Christian Wolf <[email protected]>
Fixes #1464
Signed-off-by: Nikolas Rimikis [email protected]