-
Notifications
You must be signed in to change notification settings - Fork 288
Feat: add OCS routes for mailbox and mail listing #11425
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
|
Thanks for opening your first pull request in this repository! ✌️ |
f459cda to
ba32a24
Compare
ba32a24 to
04063bb
Compare
cf675c2 to
4d368a7
Compare
| $userId, | ||
| $view | ||
| ); | ||
| return new DataResponse(json_encode($messages), Http::STATUS_OK); |
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.
json_encode should not be necessary when using DataResponse, I think 🤔
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.
You're right, removed :)
89f3d55 to
bb06df5
Compare
| * 200: Mailbox list | ||
| * 404: User was not logged in or account doesn't exist | ||
| */ | ||
| #[ApiRoute(verb: 'GET', url: '/mailbox/list')] |
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.
Nitpick: not happy about the URL design. /api/mailboxes is already taken by the internal API. How about /api/ocs/mailboxes or similar to have a new, dedicated sub path for public ocs APIs?
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.
The api/ part is left out for the OCS routes for now, but I'm open to anything here. Could put ocs instead of api, so /ocs/mailboxes/list?
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.
I'm fine with leaving out the api part for ocs. But please make it ocs/mailboxes without the list
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.
Sure, done!
Signed-off-by: Jana Peper <jana.peper@nextcloud.com>
Signed-off-by: Jana Peper <jana.peper@nextcloud.com>
bb06df5 to
a76eb7a
Compare
Signed-off-by: Jana Peper <jana.peper@nextcloud.com>
a76eb7a to
f6b793d
Compare
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.
👍 otherwise :)
Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com> Signed-off-by: janepie <49834966+janepie@users.noreply.github.com>
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.
Looks good! Thank you!
adds OCS routes to