Skip to content

fix: do not add organizer as attendee and set status #14664

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
SebastianKrupinski merged 1 commit into from
Mar 31, 2025
Merged

fix: do not add organizer as attendee and set status #14664

SebastianKrupinski merged 1 commit into from
Mar 31, 2025

Conversation

@SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Mar 15, 2025 •
edited by nickvergessen
Loading

☑️ Resolves

Event status should be set on new events, the organizer should also not be added as a participant.

🏁 Checklist

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@SebastianKrupinski SebastianKrupinski self-assigned this Mar 15, 2025
This was referenced Mar 15, 2025
Merged
Closed
@nickvergessen nickvergessen added feature: api 🛠️ OCS API for conversations, chats and participants feature: meetings 📅 Covering the webinary usecase incl. Lobby bug labels Mar 18, 2025
@nickvergessen nickvergessen added this to the 🪺 Next Major (32) milestone Mar 18, 2025
@nickvergessen
Copy link
Member

nickvergessen commented Mar 18, 2025

/backport to stable31

miaulalala
miaulalala requested changes Mar 18, 2025
View reviewed changes
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.

Can I ask why CONFIRMED? Theoretically, it should be a PARTSTAT NEEDS-ACTION. The STATUS should not be set for the user without their input.

@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Mar 18, 2025 •
edited
Loading

Can I ask why CONFIRMED? Theoretically, it should be a PARTSTAT NEEDS-ACTION. The STATUS should not be set for the user without their input.

Morning, the status is for the event not for the attendees, "PARTSTAT NEEDS-ACTION" this would be for the attendees. The event STATUS and Organizer status should be set to confimed if the organizer is creating the event.

@miaulalala
Copy link
Contributor

miaulalala commented Mar 18, 2025

Can I ask why CONFIRMED? Theoretically, it should be a PARTSTAT NEEDS-ACTION. The STATUS should not be set for the user without their input.

Morning, the status is for the event not for the attendees, "PARTSTAT NEEDS-ACTION" this would be for the attendees. The event STATUS and Organizer status should be set to confimed if the organizer is creating the event.

damn, right. Looks good!

miaulalala
miaulalala approved these changes Mar 18, 2025
View reviewed changes
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.

Static analysis is complaining since the upstream PR is not merged. Please rebase when it's done, then you can merge.

nickvergessen
nickvergessen reviewed Mar 18, 2025
View reviewed changes
lib/Controller/RoomController.php Outdated
$eventBuilder->setOrganizer($user->getEMailAddress(), $user->getDisplayName() ?: $this->userId);
$eventBuilder->setStartDate($startDate);
$eventBuilder->setEndDate($endDate);
$eventBuilder->setStatus('CONFIRMED');
Copy link
Member

@nickvergessen nickvergessen Mar 18, 2025

Choose a reason for hiding this comment

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

Suggested change
$eventBuilder->setStatus('CONFIRMED');
if (method_exists($eventBuilder, 'setStatus')) {
$eventBuilder->setStatus('CONFIRMED');
}

if we want to backport

@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-status-and-organizer branch 3 times, most recently from fbbbb41 to ae9564b Compare March 29, 2025 22:18
nickvergessen
nickvergessen reviewed Mar 30, 2025
View reviewed changes
lib/Controller/RoomController.php
$eventBuilder->setOrganizer($user->getEMailAddress(), $user->getDisplayName() ?: $this->userId);
$eventBuilder->setStartDate($startDate);
$eventBuilder->setEndDate($endDate);
$eventBuilder->setStatus(CalendarEventStatus::CONFIRMED);
Copy link
Member

@nickvergessen nickvergessen Mar 30, 2025

Choose a reason for hiding this comment

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

Needs to be wrapped with method exists at least on the backport

Copy link
Contributor Author

@SebastianKrupinski SebastianKrupinski Mar 31, 2025

Choose a reason for hiding this comment

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

I saw your previous comment and was going to ask you why this would be needed since the talk app tracks NC version for version, but then I realized that the apps can be upgraded before the server. 💡

I think wrapping it in the back port would be cleaner, that way the code from this version on will be cleaner. What would you prefer?

@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-status-and-organizer branch from ae9564b to f7c26ef Compare March 31, 2025 17:20
@SebastianKrupinski SebastianKrupinski merged commit 3f421e2 into main Mar 31, 2025
78 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/noid-status-and-organizer branch March 31, 2025 17:32
@backportbot backportbot bot mentioned this pull request Mar 31, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@miaulalala miaulalala miaulalala approved these changes

Assignees

@SebastianKrupinski SebastianKrupinski

Labels

3. to review bug feature: api 🛠️ OCS API for conversations, chats and participants feature: meetings 📅 Covering the webinary usecase incl. Lobby

Projects

None yet

Milestone

v22.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

Can not accept calendar invite

4 participants

@SebastianKrupinski @nickvergessen @miaulalala