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 widget #2414

Merged
merged 1 commit into from
Sep 1, 2020
Merged

Dashboard widget #2414

merged 1 commit into from
Sep 1, 2020

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jul 22, 2020

No events No events today Events List with ToDo
F1BAAB3B-17E6-4D4E-99D1-D725AEB2E2C0 5E4359A5-5180-40B9-AE19-5B79B8085847 95FB5E86-F5E9-4F43-8F16-6F1EB67786D0 95C015E8-7EA2-413E-B9B3-C0BD94CB5A14

Description

This PR implements a basic dashboard widget for Nextcloud 20.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How to test / use your changes?

UI Changes

Checklist:

  • My code follows the style guidelines of this project
  • I signed off my changes (git commit -sm "Your commit message")
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

ToDo

  • Check for backward compatibility on older Nextcloud versions
  • Fetch a list of upcoming events from the server
  • Determine which information we should show per event

@codecov
Copy link

codecov bot commented Jul 22, 2020

Codecov Report

Merging #2414 into master will decrease coverage by 0.04%.
The diff coverage is 27.43%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2414      +/-   ##
============================================
- Coverage     30.00%   29.95%   -0.05%     
- Complexity      103      116      +13     
============================================
  Files           149      153       +4     
  Lines          5356     5468     +112     
  Branches        802      811       +9     
============================================
+ Hits           1607     1638      +31     
- Misses         3749     3830      +81     
Flag Coverage Δ Complexity Δ
#javascript 24.50% <0.00%> (-0.39%) 0.00 <0.00> (ø)
#php 94.15% <88.57%> (-0.26%) 116.00 <13.00> (+13.00) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
lib/AppInfo/Application.php 0.00% <0.00%> (ø) 3.00 <2.00> (+2.00)
src/dashboard.js 0.00% <0.00%> (ø) 0.00 <0.00> (?)
src/views/Dashboard.vue 0.00% <0.00%> (ø) 0.00 <0.00> (?)
lib/Dashboard/CalendarWidget.php 100.00% <100.00%> (ø) 8.00 <8.00> (?)
lib/Service/JSDataService.php 100.00% <100.00%> (ø) 3.00 <3.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25e8523...eb1c1aa. Read the comment docs.

@juliusknorr
Copy link
Member Author

@georgehrke Is there any way we can query the DAV endpoint for something like "the next 10 upcoming events"? Otherwise we probably should have a new HTTP endpoint for that where we collect the relevant list. This would have the benefit that we could also provide the list through the initial state api and reduce loading time when the user sees no information being available in the frontend.

@georgehrke
Copy link
Member

No, there is no such endpoint. The problem here are recurring events. There is no way for us to easily query "the next 10 events" or something similar. (Neither via CalDAV nor from the database for that matter)

If you send a calendar-query with a time-range, it will return:

  • all non-recurring events in that time-range
  • all recurring events:
    • where the first occurrence is earlier than the end-date
    • and where the last occurrence is later than the start-date

The server doesn't actually check whether the recurring event is occurring in that time-range. For example, if you query all events for June, the server will just return all birthday events, even if the person was not born in June.

@georgehrke

This comment has been minimized.

@jancborchardt jancborchardt added 2. developing Work in progress enhancement New feature request labels Aug 11, 2020

$container = $this->getContainer();

// TODO: Migrate to new bootstrap once Calendar supports only Nextcloud 20+
Copy link
Member

Choose a reason for hiding this comment

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

We can just bump to 2.1 and limit it to Nc 20+

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me 👍

Copy link
Member

Choose a reason for hiding this comment

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

@tcitworld Do you have any objections here?
I would release this as part of Calendar 2.1 and we can always backport bugfixes to 2.0.x when necessary :)

Copy link
Member

@tcitworld tcitworld Aug 19, 2020

Choose a reason for hiding this comment

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

Suits me. There's at least #2286 and #2275 that would be nice to have in a close 2.0.x release (maybe released before 20 gets out with 2.1?).

@georgehrke
Copy link
Member

FC94FEC9-188B-4091-B210-2822AD5EDC48

@georgehrke
Copy link
Member

@jancborchardt I also added a small colored circle with the calendar color.

src/views/Dashboard.vue Outdated Show resolved Hide resolved
src/views/Dashboard.vue Outdated Show resolved Hide resolved
@jancborchardt

This comment has been minimized.

@juliusknorr
Copy link
Member Author

@georgehrke There seems to be an issue with having only the default personal calendar, in that case there are no requests sent and no events shown. When adding a second calendar only the events of that are shown.

@georgehrke
Copy link
Member

@juliushaertl Will check that. Was that calendar enabled?

@jancborchardt
Copy link
Member

And an additional thing: If there are no events left for today, can we have an entry saying "All events done for today!" – it could even take 2 rows so it’s clear that today is done. Otherwise you have to check the subline to make sure you got it all.

@georgehrke good example of what I meant by this is in the Talk widget: nextcloud/spreed#3890 (comment)

@georgehrke
Copy link
Member

@jancborchardt Please see the updated screenshots in the original post

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

@jancborchardt Please see the updated screenshots in the original post

Super super super nice! :) Getting 2 issues though:

@georgehrke
Copy link
Member

Requires #2483

@georgehrke georgehrke force-pushed the enh/dashboard branch 2 times, most recently from 0f4d1aa to 2abed44 Compare August 21, 2020 14:29
@georgehrke georgehrke marked this pull request as ready for review August 21, 2020 14:31
@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 21, 2020
src/views/Dashboard.vue Outdated Show resolved Hide resolved
@georgehrke
Copy link
Member

I would like to get #2498 in first, then we can use the icon-font i added for tasks:
2B83E566-3290-490E-A7E5-446A21FCA998

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Copy link
Member

@georgehrke georgehrke left a comment

Choose a reason for hiding this comment

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

Good to merge from my side 👍

@georgehrke
Copy link
Member

All remarks have been addressed.
Let's get this in, we can always fix smaller issues later.

@georgehrke georgehrke merged commit 9594a30 into master Sep 1, 2020
@delete-merged-branch delete-merged-branch bot deleted the enh/dashboard branch September 1, 2020 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants