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

Dashboard API for clients #26430

Merged
merged 2 commits into from
Jun 16, 2021
Merged

Dashboard API for clients #26430

merged 2 commits into from
Jun 16, 2021

Conversation

julien-nc
Copy link
Member

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 of WidgetItem.

Here are the changes made to GitHub integration dashboard widget to implement this client API:
nextcloud/integration_github@e888d8f#diff-322e5fa76010510d00333c44839783ab8b832362c8719b4b54ae08a6b99e853bR44

Copy link
Member

@juliushaertl juliushaertl left a 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.

@juliushaertl juliushaertl added the pending documentation This pull request needs an associated documentation update label Apr 8, 2021
@juliushaertl
Copy link
Member

Once merged this should also be documented in https://docs.nextcloud.com/server/21/developer_manual/digging_deeper/dashboard.html

@julien-nc julien-nc force-pushed the enh/dashboard-api branch 2 times, most recently from c40f928 to 4e4d921 Compare April 8, 2021 10:59
@julien-nc
Copy link
Member Author

@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) {
Copy link
Member

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

Copy link
Member Author

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@juliushaertl juliushaertl left a 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 👍

@MorrisJobke
Copy link
Member

🏓

@MorrisJobke MorrisJobke mentioned this pull request May 26, 2021
98 tasks
@blizzz blizzz mentioned this pull request Jun 2, 2021
57 tasks
@skjnldsv
Copy link
Member

skjnldsv commented Jun 2, 2021

Moved to 23

@julien-nc
Copy link
Member Author

@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 ci/drone/pr fails...

@juliushaertl
Copy link
Member

From https://drone.nextcloud.com/nextcloud/server/6078/1/3

The autoloaders are not up to date
Please run: bash build/autoloaderchecker.sh
And commit the result

@juliushaertl
Copy link
Member

Other drone failures seem unrelated

Copy link
Member

@MorrisJobke MorrisJobke left a 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) 👍

@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
59 tasks
@juliushaertl
Copy link
Member

🏓 @eneiluj

@skjnldsv
Copy link
Member

@artonge could you have a look at this and see if you can find any blockers? :)

@artonge
Copy link
Contributor

artonge commented Jun 14, 2021

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 😉

Copy link
Contributor

@artonge artonge left a 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 😞 .

lib/public/Dashboard/IAPIWidget.php Show resolved Hide resolved
lib/public/Dashboard/IAPIWidget.php Show resolved Hide resolved
lib/public/Dashboard/Model/WidgetItem.php Show resolved Hide resolved
lib/public/Dashboard/Model/WidgetItem.php Show resolved Hide resolved
Julien Veyssier and others added 2 commits June 15, 2021 21:34
…dget interface

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Copy link
Member

Rebased and pushed the updated autoloader.

@juliushaertl juliushaertl added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 15, 2021
@juliushaertl juliushaertl merged commit 51add75 into master Jun 16, 2021
@juliushaertl juliushaertl deleted the enh/dashboard-api branch June 16, 2021 09:34
@blizzz blizzz mentioned this pull request Jun 16, 2021
45 tasks
@ChristophWurst ChristophWurst removed the pending documentation This pull request needs an associated documentation update label Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants