Skip to content

fix(imip): dont compare events for the reply message #51131

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2025
Merged

fix(imip): dont compare events for the reply message #51131

merged 1 commit into from
Jun 5, 2025

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Feb 28, 2025 •
edited
Loading

Summary

TODO

  • ...

Checklist

@hamza221 hamza221 added bug 3. to review Waiting for reviews feature: dav feature: caldav Related to CalDAV internals labels Feb 28, 2025
@hamza221 hamza221 self-assigned this Feb 28, 2025
SebastianKrupinski
SebastianKrupinski reviewed Feb 28, 2025
View reviewed changes
apps/dav/lib/CalDAV/Schedule/IMipPlugin.php Outdated
@@ -185,7 +185,8 @@ public function schedule(Message $iTipMessage) {
switch (strtolower($iTipMessage->method)) {
case self::METHOD_REPLY:
$method = self::METHOD_REPLY;
$data = $this->imipService->buildBodyData($vEvent, $oldVevent);
$organizerEvent = $this->imipService->getOrganizerVEvent($vEvent->uid, $iTipMessage->recipient);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, you can't just pull the event by the recipient, This will fail, for external organizers

@susnux susnux added this to the Nextcloud 32 milestone Mar 2, 2025
@hamza221
Copy link
Contributor Author

hamza221 commented Mar 3, 2025

@SebastianKrupinski would it make sense to keep this even that it only works if the organiser is a local user?
I mean check if he is and search for the event. Sent potentially wrong message in the other case? Would be better than nothing no ?

@SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski would it make sense to keep this even that it only works if the organiser is a local user? I mean check if he is and search for the event. Sent potentially wrong message in the other case? Would be better than nothing no ?

I don't think having different code paths for internal and external attendees is a good idea, just makes it too complex, the only thing I would keep is the separate method for the reply message, as the reply does not need event comparison, and all the extra information that is normally generate

@hamza221 hamza221 marked this pull request as draft March 3, 2025 13:52
@hamza221 hamza221 force-pushed the fix/reply-message branch 2 times, most recently from 2e9dfc5 to d949cca Compare March 26, 2025 14:06
@hamza221 hamza221 marked this pull request as ready for review March 26, 2025 14:07
@hamza221 hamza221 force-pushed the fix/reply-message branch from d949cca to cafd8ad Compare March 26, 2025 14:07
@hamza221 hamza221 changed the title fix(imip): dont propagate attendee changes in reply message fix(imip): dont compare events for the reply message Mar 26, 2025
@hamza221 hamza221 force-pushed the fix/reply-message branch from cafd8ad to ed1bfe7 Compare June 5, 2025 10:31
@hamza221 hamza221 enabled auto-merge June 5, 2025 10:31
st3iny
st3iny approved these changes Jun 5, 2025
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works.

@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 Jun 5, 2025
@st3iny
Copy link
Member

st3iny commented Jun 5, 2025

/backport to stable30

@st3iny
Copy link
Member

st3iny commented Jun 5, 2025

/backport to stable31

@hamza221 hamza221 merged commit d6a53ce into master Jun 5, 2025
203 of 205 checks passed
@hamza221 hamza221 deleted the fix/reply-message branch June 5, 2025 12:14
@backportbot backportbot
Copy link

backportbot bot commented Jun 5, 2025

The backport to stable31 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable31
git pull origin stable31

# Create the new backport branch
git checkout -b backport/51131/stable31

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick ed1bfe70

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/51131/stable31

Error: Failed to push branch backport/51131/stable31: remote: Invalid username or password.
fatal: Authentication failed for 'https://github.com/nextcloud/server.git/'

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot backportbot bot mentioned this pull request Jun 5, 2025
Merged
@st3iny
Copy link
Member

st3iny commented Jun 5, 2025

/backport to stable31

@backportbot backportbot bot mentioned this pull request Jun 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@st3iny st3iny st3iny approved these changes

@SebastianKrupinski SebastianKrupinski SebastianKrupinski approved these changes

@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

Assignees

@hamza221 hamza221

Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: caldav Related to CalDAV internals feature: dav
Projects
None yet
Milestone
Nextcloud 32
Development

Successfully merging this pull request may close these issues.

Changes by attendee propagating to organiser
4 participants
@hamza221 @SebastianKrupinski @st3iny @susnux