Feat/add calendar delegation features#8085
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
da7ae86 to
654e770
Compare
kra-mo
left a comment
There was a problem hiding this comment.
From the design side:
"Select a user" and "Search for a user…" are redundant, it's fine to just have the second if it's baked into the component anyway, so without the title. The input field should also be full-width, and Add should be a primary button.
I wonder why delegation should be its own item in the sidebar instead of just a section in Settings? I'd rather have it just there. And at that point, have + Add delegate be full-width as well.
0dd4839 to
f50f37d
Compare
kra-mo
left a comment
There was a problem hiding this comment.
Looks good from the design side :)
|
Tested. Found error when delegator has a shared calendar |
|
I think it's related to nextcloud/server#59232 @hamza221
|
Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
e8b2495 to
d8f14d7
Compare
|
Rebased to most recent head |
|
Accidental close |
…calendars Delegated calendars showed a permanent loading spinner because their owners' principals were never added to the principals store, so the avatar in CalendarListItem could not resolve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each delegator now gets its own "Delegated by {name}" caption above
their calendars. Grouping uses a new delegatorUrl field on the calendar
(the principal of the proxy-granting user), since the calendar's owner
can differ when the delegator only has access via a regular share.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Append "(read-only)" to the "Delegated by {name}" caption when the
delegator granted read-only proxy access, so users can tell at a
glance which delegated calendars they can edit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The counter avatar and "Delegated to you by" action text now use the delegator's principal rather than the calendar's owner, so a calendar that the delegator received via a regular share is correctly attributed to the delegator who actually granted proxy access. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In the event editor's CalendarPickerHeader and the shared
CalendarPicker, delegated calendars now display
"{name} (delegated by {delegator})" and show the delegator's avatar
(instead of the calendar owner's). The label is built as a single
string because NcActionButton only renders the first text node of its
default slot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks a lot for fixing the broken stuff @hamza221! Please don't merge this for now though because you accidentally removed the information on whether people are read/write delegates in settings, I'll re-introduce that.
EDIT: NOT TRUE
|
Revoking access also stopped working EDIT: NOT TRUE |
|
@hamza221 My bad, ignore my comments |
|
Btw sorry for the co-authored commits, I told it to commit as Assisted but apparently it ignored my, will be fixed on squash. |
There was a problem hiding this comment.
any newly created files should be in TypeScript with setup and Vue composition api
There was a problem hiding this comment.
any newly created files should be in TypeScript with setup and Vue composition api.
| const baseUrl = currentUser.url.replace(/\/?$/, '') | ||
| const proxyWriteGroupUrl = baseUrl + '/calendar-proxy-write' | ||
| const proxyReadGroupUrl = baseUrl + '/calendar-proxy-read' |
There was a problem hiding this comment.
The CDav library should be responsible for creating the specific urls the Calendar app should not need to know how to format a dav url
| [writeUrls, readUrls] = await Promise.all([ | ||
| getClient().getDelegateUrls(proxyWriteGroupUrl), | ||
| getClient().getDelegateUrls(proxyReadGroupUrl), | ||
| ]) |
There was a problem hiding this comment.
According to the description in the cdav library getDelegateUrls is to be use to retrieve all write-delegates, but you are also retrieving read delegates.
This is also in efficient, you are making two requests one for read only delegates and one for read write delegates, you should be able to do both in one request.
| const baseUrl = currentUser.url.replace(/\/?$/, '') | ||
| const proxyGroupUrl = permission === 'read' | ||
| ? baseUrl + '/calendar-proxy-read' | ||
| : baseUrl + '/calendar-proxy-write' | ||
| await getClient().addDelegate(proxyGroupUrl, principalUrl) |
There was a problem hiding this comment.
Same comment as above, this is the responsibility of the cdav library
| const baseUrl = currentUser.url.replace(/\/?$/, '') | ||
| // Find the delegate's current permission so we remove from the right group. | ||
| const existing = this.delegates.find((d) => d.url === principalUrl) | ||
| const permission = existing?.permission ?? 'write' | ||
|
|
||
| const proxyGroupUrl = permission === 'read' | ||
| ? baseUrl + '/calendar-proxy-read' | ||
| : baseUrl + '/calendar-proxy-write' | ||
| await getClient().removeDelegate(proxyGroupUrl, principalUrl) |
There was a problem hiding this comment.
Same comment as above, this is the responsibility of the cdav library
|
One last comment, this feature needs a version gate for 34 unless we are back porting all the server changes |















For #2706
COMPREHENSIVE GUIDE FOR TESTING THIS