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

Fix events being editable by invitees #3424

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Aug 20, 2021

Fixes #947

Events should be read only if invitees view them. Only the alarms can be edited.

@codecov
Copy link

codecov bot commented Aug 20, 2021

Codecov Report

Merging #3424 (7dbae9d) into master (9309fa5) will increase coverage by 1.69%.
The diff coverage is 0.00%.

❗ Current head 7dbae9d differs from pull request most recent head 74b78b9. Consider uploading reports for the commit 74b78b9 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3424      +/-   ##
============================================
+ Coverage     27.70%   29.39%   +1.69%     
- Complexity      123      317     +194     
============================================
  Files           165      219      +54     
  Lines          6018     7396    +1378     
  Branches        877      919      +42     
============================================
+ Hits           1667     2174     +507     
- Misses         4351     5222     +871     
Flag Coverage Δ
javascript 20.70% <0.00%> (-1.64%) ⬇️
php 67.51% <ø> (-27.11%) ⬇️

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

Impacted Files Coverage Δ
src/components/Editor/Invitees/InviteesList.vue 0.00% <ø> (ø)
...rc/components/Editor/Invitees/InviteesListItem.vue 0.00% <ø> (ø)
src/components/Editor/SaveButtons.vue 0.00% <0.00%> (ø)
src/mixins/EditorMixin.js 0.00% <0.00%> (ø)
src/views/EditSidebar.vue 0.00% <ø> (ø)
src/views/EditSimple.vue 0.00% <ø> (ø)
src/main.js 0.00% <0.00%> (ø)
src/store/index.js 0.00% <0.00%> (ø)
src/store/calendars.js 0.00% <0.00%> (ø)
src/views/Calendar.vue 0.00% <0.00%> (ø)
... and 58 more

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 9309fa5...74b78b9. Read the comment docs.

@tcitworld
Copy link
Member

I didn't follow that closely… does this mean as a sharee I can no longer edit events inside a calendar that is shared with me?

Shouldn't it only affect the invites section (because of https://github.com/nextcloud/server/issues/26668 ?)

@ChristophWurst
Copy link
Member

I didn't follow that closely… does this mean as a sharee I can no longer edit events inside a calendar that is shared with me?

Shouldn't it only affect the invites section (because of https://github.com/nextcloud/server/issues/26668 ?)

Not quite. This PR doesn't address events on shared calendars but invites to events in your own calendar, where the invitees receive a copy of the even in their personal calendar. That event is currently editable, though the changes go nowhere. With @st3iny's patch the event copy is read-only and only reminders can be edited.

@tcitworld
Copy link
Member

This PR doesn't address events on shared calendars but invites to events in your own calendar

I see that that's the intention, but I fail to see how it wouldn't affect an event (of which I'm not the organizer) I have in my calendar which someone shared to me.

@ChristophWurst
Copy link
Member

I fail to see how it wouldn't affect an event (of which I'm not the organizer) I have in my calendar which someone shared to me

@st3iny you tested that as well, right?

@st3iny
Copy link
Member Author

st3iny commented Aug 23, 2021

Yes, this introduces a new edge case.

  1. Person 1 creates a calendar and shares it with Person 2 (with edit rights).
  2. Person 1 creates an event in said calendar.
  3. Person 2 invites Person 1 to the event.
  4. The event will be read only for Person 1 even though he created it.

IMO, this bug is out of scope for this PR because we're currently not tracking the owner of an event.

@ChristophWurst
Copy link
Member

we forgot about this one. @st3iny please rebase and then we merge & backport :)

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

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

LGTM

@ChristophWurst
Copy link
Member

@miaulalala could you rebase and test on top of latest master?

@miaulalala miaulalala force-pushed the fix/947/event-read-only-as-attendee branch from af27697 to 105fe19 Compare November 22, 2021 16:04
@miaulalala
Copy link
Contributor

Testing looks good, just some superflous stuff I merged might still be in there. @st3iny, if you could look through it again?

@st3iny
Copy link
Member Author

st3iny commented Nov 23, 2021

The changes to InviteeList* might need to be ported to ResourceList*. I'll give it a final test run.

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the fix/947/event-read-only-as-attendee branch from c441c33 to 74b78b9 Compare November 23, 2021 15:11
@st3iny
Copy link
Member Author

st3iny commented Nov 23, 2021

Ready to merge

@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 Nov 23, 2021
@miaulalala miaulalala mentioned this pull request Nov 25, 2021
@st3iny
Copy link
Member Author

st3iny commented Dec 1, 2021

I actually managed to fix the edge case I mentioned way back then.

Turns out I was wrong about tracking the owner. While we can't track the owner of an event we can track the owner of its calendar. I added an extra condition to always allow edits to the owner of the calendar.

EDIT: Turns out that introduces a new edge case because the events you are invited to are added to your own calendars -> they are not shared with you. I think we really need to track the origin of an event to be able to fix the initial edge case.

@st3iny st3iny force-pushed the fix/947/event-read-only-as-attendee branch from 7dbae9d to 74b78b9 Compare December 1, 2021 23:32
@ChristophWurst ChristophWurst added this to the v3.1.0 milestone Dec 17, 2021
@ChristophWurst
Copy link
Member

/backport to stable3.0

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 bug Feature: Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anyone can become event's organiser and then do anything with event, even cancel it
4 participants