-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Dashboard API for clients #26430
Dashboard API for clients #26430
Conversation
f38c81b
to
e44d9b8
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.
General structure seems fine, there are some php-cs and psalm failures besides my comments.
Once merged this should also be documented in https://docs.nextcloud.com/server/21/developer_manual/digging_deeper/dashboard.html |
c40f928
to
4e4d921
Compare
@juliushaertl Thanks for the review. Link added, php-cs/psalm error fixed and rebased on master. |
$widgets = $this->dashboardManager->getWidgets(); | ||
foreach ($widgets as $widget) { | ||
if ($widget instanceof IAPIWidget && in_array($widget->getId(), $userLayout)) { | ||
$items[$widget->getId()] = array_map(function (WidgetItem $item) { |
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 shouldn't be a need or the mapping if the WidgetItem implements JsonSerializable
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.
What do you mean?
How would you serialize each item?
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 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.
Some smaller additional comments but otherwise 👍
4e4d921
to
69fc91b
Compare
🏓 |
Moved to 23 |
69fc91b
to
af47bfb
Compare
af47bfb
to
3f8dff7
Compare
@juliushaertl @MorrisJobke @skjnldsv Rebased on master, affected milestone 22 again (it does not break anything and it's ready). I don't know why the |
From https://drone.nextcloud.com/nextcloud/server/6078/1/3
|
Other drone failures seem unrelated |
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.
Code looks good (beside the one nitpick) 👍
🏓 @eneiluj |
@artonge could you have a look at this and see if you can find any blockers? :) |
The code scanning warnings should get fixed. And it would be saner with Julius' suggestion for the JSON serialisation. Beside that, LGTM. But I might not be the best juge for php code yet 😉 |
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.
Oh nop, comments are wrong 😞 .
…dget interface Signed-off-by: Julien Veyssier <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
3f8dff7
to
08899c8
Compare
Rebased and pushed the updated autoloader. |
This brings a new dashboard API endpoint which provides a standardized way to get enabled widgets content for Nextcloud clients. For the moment, only widgets displaying an ordered list of items can be accessed by this API. It could be extended to other kinds of content later.
The new
WidgetItem
class is a generic representation of one widget element.The initial IWidget interface can still be used by widgets. The new IAPIWidget interface just adds a
getItems
method which returns an array ofWidgetItem
.Here are the changes made to GitHub integration dashboard widget to implement this client API:
nextcloud/integration_github@e888d8f#diff-322e5fa76010510d00333c44839783ab8b832362c8719b4b54ae08a6b99e853bR44