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

feat(dashboard): implement widget item api v2 #39937

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Aug 17, 2023

  • Resolves: none

Summary

This API enables the dashboard to render all widgets from the API data alone without having apps to provide their own bundles. This saves a lot of traffic and execution time as a lot less javascript has to be parsed on the frontend.

This is already done on the clients. However, the dashboard requires some extra logic and data as its widgets tend to be more sophisticated. To prevent breaking all apps and clients new API (v2) was introduced allowing each app and each client to migrate at their own pace.

Profiling results

results

Perceived loading time: Time from hitting refresh until all dashboard widgets finished loading.
No cache: Simulates a new visitor or a visitor returning after a long time. Cache is disabled in Chrome DevTools to force the browser to redownload all assets.
Cache: Simulates a user that often uses Nextcloud. Browser cache is enabled.

Tests were run on the same instance with exactly the same calendar, mail and spreed data etc.

Before

Without cache

Time until all widgets finised loading: 4284 ms

Chrome network stats:

151 requests
36.8 MB transferred
36.8 MB resources
DOMContentLoaded: 2.09 s
Load: 3.72 s

With cache

Time until all widgets finised loading: 3836 ms

Chrome network stats:

190 requests
126 kB transferred
36.6 MB resources
DOMContentLoaded: 1.34 s
Load: 2.76 s

After

Without cache

Time until all widgets finished loading: 2995 ms

Chrome network stats:

111 requests
29.0 MB transferred
28.9 MB resources
DOMContentLoaded: 1.63 s
Load: 3.05 s

With cache

Time until all widgets finished loading: 1500 ms

Chrome network stats:

132 requests
75.8 kB transferred
28.9 MB resources
DOMContentLoaded: 919 ms
Load: 1.53 s

TODO

  • ...

Checklist

@st3iny st3iny added 2. developing Work in progress pending documentation This pull request needs an associated documentation update performance 🚀 feature: dashboard labels Aug 17, 2023
@st3iny st3iny self-assigned this Aug 17, 2023
foreach ($widgets as $widget) {
if ($widget instanceof IAPIWidgetV2) {
$items[$widget->getId()] = $widget
->getItemsV2($this->userId, $sinceIds[$widget->getId()] ?? null, $limit)

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of OCP\Dashboard\IAPIWidgetV2::getItemsV2 cannot be null, possibly null value provided
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

API changes LGTM

@st3iny st3iny marked this pull request as ready for review August 18, 2023 13:38
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 18, 2023
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Needs a rebase, but otherwise seems fine and functional

@st3iny st3iny added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 21, 2023
@st3iny st3iny added this to the Nextcloud 28 milestone Aug 21, 2023
@st3iny
Copy link
Member Author

st3iny commented Aug 21, 2023

/backport 6982597 to stable27

st3iny and others added 2 commits August 22, 2023 08:36
This API enables the dashboard to render all widgets from the API data
alone without having apps to provide their own bundles. This saves a lot
of traffic and execution time as a lot less javascript has to be parsed
on the frontend.

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member

Conflicting files
dist/settings-users-8351.js
dist/settings-users-8351.js.map
dist/settings-vue-settings-apps-users-management.js
dist/settings-vue-settings-apps-users-management.js.map 

Rebasing

@nickvergessen nickvergessen merged commit 5234807 into master Aug 22, 2023
36 of 38 checks passed
@nickvergessen nickvergessen deleted the feat/dashboard/item-api-v2 branch August 22, 2023 07:35
@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@AndyScherzinger
Copy link
Member

/backport 6982597 to stable27

@nickvergessen
Copy link
Member

Will have to do it manually, the OpenAPI stuff does not exist on stable27

@nickvergessen
Copy link
Member

PS I'm on it aklready

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

nickvergessen added a commit to nextcloud/spreed that referenced this pull request Aug 22, 2023
Ref nextcloud/server#39937

Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen added a commit to nextcloud/spreed that referenced this pull request Aug 22, 2023
Ref nextcloud/server#39937

Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen added a commit to nextcloud/spreed that referenced this pull request Aug 22, 2023
Ref nextcloud/server#39937

Signed-off-by: Joas Schilling <coding@schilljs.com>
backportbot-nextcloud bot pushed a commit to nextcloud/spreed that referenced this pull request Aug 22, 2023
Ref nextcloud/server#39937

Signed-off-by: Joas Schilling <coding@schilljs.com>
@st3iny st3iny removed backport-request pending documentation This pull request needs an associated documentation update labels Dec 4, 2023
@miaulalala miaulalala mentioned this pull request Feb 13, 2024
5 tasks
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 feature: dashboard performance 🚀
Projects
Development

Successfully merging this pull request may close these issues.

5 participants