-
Notifications
You must be signed in to change notification settings - Fork 279
feat: allow calendar-wide only-availability share setting #7556
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @MiragonMx Thank you for the contribution and congratulation on your first contribution! I will have a closer look at your PR and what you are trying to achieve later this week |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7556 +/- ##
==========================================
- Coverage 13.02% 13.00% -0.02%
==========================================
Files 224 224
Lines 11634 11653 +19
Branches 2797 2814 +17
==========================================
+ Hits 1515 1516 +1
- Misses 9759 9776 +17
- Partials 360 361 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
As discussed in the related issue (#6247), I suppose I still have to establish the "link" to the existing CalDAV property
I'm not sure, however, where that happens exactly or if my variable name choice would simply have to match the |
|
Also, I suppose my dev env missed some linters, I'll try to fix that - same for commit signoff |
Linter DCO |
First shot at implementing the `CALDAV:read-free-busy` privilege setting. Should allow sharing a whole calendar with only availability knowledge (without event details) instead of setting this flag for each event individually. Signed-off-by: MiragonMx <80544476+MiragonMx@users.noreply.github.com>
1c411e8 to
514abbb
Compare
Tried at least fixing these obvious blockers, didn't have time to look into it more until now |
|
Hi @MiragonMx So I looked more in to this. But I have some bad news, the setting you are trying to use (CALDAV:read-free-busy) is an ACL for the Free/Busy view this does not apply to shared calendars. This is not the same as the issue ticket you linked, the ticket talks about shared calendars. On the PR front, yes the setting is available in the calendar setting modal, but the setting does not get pushed to the server (you can examine the network requests in the browser). To make this work, you need to make the front end push the setting to the server (this might require modification of the nextcloud/cdav-library), then the server code would need to be modified to produce the correct returns when the setting is set. |
|
Hi @SebastianKrupinski , first of all thanks for looking into this and coming back to me so quickly!
Ahh I see. I got the ACL setting from the issue thread (as well as the 'template PR' that I tried to mimic) and thought this would work for shared calendars as well.
That is what I wondered - I didn't have time to look into the traffic and server side before, but it would have seemed weird to me if this had been enough as I didn't see where I would have changed server side settings - once again, this was mainly based on the other PR on modal transparency, I tried to get this off the ground quickly without digging too deep into the codebase, but this obviously fell short. What I am unsure about now: do you think that pushing this setting to server side with the corresponding response would be sensible/feasible? And would that still only make this option persistent for regular calendar objects or could that work for shared calendars as well with the server side changes? I'll try to look into this more soon, it does not surprise me that this does seem to be at least a bit more complex and time consuming than I had initially hoped (though that would have been nice of course :D )... Thanks again for the feedback and review! |
As this setting is meant for the Free/Busy, it would only affect the free busy view. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Hi again, got reminded by the bot... I filled out the form but can't seem to remove the assigned label. |
|
@MiragonMx I changed the label for you to developing |
Fixes #6247
(Or at least tries do do that)
First shot at implementing the
CALDAV:read-free-busyprivilege setting.This should allow sharing a whole calendar with only availability knowledge (without event details) instead of setting this flag for each event individually.
Full disclaimer: this is my first contribution to nextcloud. I don't have much experience with typescript or vue. I've tried my best and have developed my commit based on the PR #6169 . I'm not exactly sure how to test this/if my dev environment that I threw together fully allows for this and I had very lacking network while developing this on public transport.. If there are any pointers/errors/missing things, please point them out or take this over - I only thought I'd kickstart this when I found the referenced issue yesterday and thought this might be a rather minor fix that would benefit me a lot! :)