Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
7856804 to
ae2ecc3
Compare
| private readonly IL10N $l10n, | ||
| private readonly IURLGenerator $urlGenerator, | ||
| private readonly IManager $calendarManager, | ||
| private CalDavBackend $calDavBackend, |
There was a problem hiding this comment.
This should be made backend agnostic, and use OCP\ICalendar\IManager::getCalendarsForPrincipal in the getCalendar method, and OCP\Calendar\ICalendar\search in the getEventData method.
There was a problem hiding this comment.
Can you help me to use ICalendar::search() for finding the event? It seems like it doesn't allow to filter by event URI (e.g. 45C2F160-BA74-4910-B302-79356C24A458.ics). So my best bet would be to search for all events in the calendar and then iterate through them to get the one with the matching URI:
$event = null;
foreach ($calendar->search('') as $result) {
if (($result['uri'] ?? null) === $eventFile) {
$event = $result;
break;
}
}This works but doesn't seem a good idea performance-wise. Or is it fine to do it that way?
There was a problem hiding this comment.
I mistook the object uri and the object uid, sorry about that.
You can call search('', [], ['uid' => $uid]);, but the object uri is not always uid.ics, so this won't work.
I think we could add a modification in the calendar backend to allow to also do search by object uri. I don't know of any implementation using ICalendar::search (or interfaces that extend it) at the moment, so it shouldn't be a problem.
https://github.com/nextcloud/server/blob/7cdbb38d526edc2c32240fffb64b035d5dc0cb35/apps/dav/lib/CalDAV/CalDavBackend.php#L2061-L2063
There was a problem hiding this comment.
Good idea, I did this now: nextcloud/server#59142
Tested with this PR (adjusted the search() parameters) and works well.
GVodyanov
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Please use our CSS variables instead of custom values for padding/margin/gap/etc.
You can check them out here https://docs.nextcloud.com/server/latest/developer_manual/html_css_design/css.html
Fixes: #7104 Signed-off-by: Jonas <jonas@freesources.org>
|
I realized another problem: calendar event URLs (e.g. Is this a bug, or are these URLs meant to be ephemeral? Is there something like a persistent URL to an event yet at all? |
Signed-off-by: Jonas <jonas@freesources.org>
ae2ecc3 to
fc255c2
Compare
It might be related to the event occurrence added at the end ( |
There's no route for URLs without the event occurrence part at the end, so unfortunately this leads to "Page not found" 😞 |
Fixes: #7104
Screenshot