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

Rename setting 'sendEventRemindersToSharedGroupMembers' -> 'sendEventRemindersToSharedUsers'. #31660

Conversation

dzatoah
Copy link
Contributor

@dzatoah dzatoah commented Mar 22, 2022

@tcitworld #31337 (comment):

Of course I forgot to say we should rename sendEventRemindersToSharedGroupMembers to sendEventRemindersToSharedUsers, as the share can be done with groups as well as direct users.

@dzatoah
Copy link
Contributor Author

dzatoah commented Mar 22, 2022

In which Nextcloud version will this feature come later?

@ChristophWurst
Copy link
Member

24

@dzatoah dzatoah force-pushed the pr/rename-sendEventRemindersToSharedGroupMembers-to-sendEventRemindersToSharedUsers branch from 40f3bc4 to cc8efc8 Compare March 23, 2022 10:48
@blizzz blizzz added this to the Nextcloud 25 milestone Apr 21, 2022
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@szaimen
Copy link
Contributor

szaimen commented Apr 17, 2023

Hi @dzatoah can you please fix the conflicts so that we can merge this? Thanks in advance! :)

@szaimen szaimen added the 2. developing Work in progress label Apr 17, 2023
@skjnldsv skjnldsv mentioned this pull request May 3, 2023
@dzatoah dzatoah closed this May 4, 2023
@dzatoah dzatoah force-pushed the pr/rename-sendEventRemindersToSharedGroupMembers-to-sendEventRemindersToSharedUsers branch from cc8efc8 to 1a00d99 Compare May 4, 2023 11:03
…RemindersToSharedUsers'.

Signed-off-by: Daniel Teichmann <daniel.teichmann@das-netzwerkteam.de>
@szaimen szaimen reopened this May 4, 2023
@dzatoah
Copy link
Contributor Author

dzatoah commented May 4, 2023

Thanks, @szaimen. I was confused af for a moment there...

@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 4, 2023
@szaimen szaimen enabled auto-merge May 4, 2023 13:28
@szaimen szaimen merged commit a1ed1db into nextcloud:master May 5, 2023
@ChristophWurst
Copy link
Member

@dzatoah @szaimen what's the upgrade path? If this went into 24 it would be fine but now there is a good chance there are instances with a setting for sendEventRemindersToSharedGroupMembers. We have to migrate the old setting over to the new name.

@szaimen
Copy link
Contributor

szaimen commented May 5, 2023

@dzatoah would you be up for creating the migration from the old to the new setting? :)

@dzatoah
Copy link
Contributor Author

dzatoah commented May 5, 2023

I'm sorry, but I have to decline.

@szaimen
Copy link
Contributor

szaimen commented May 5, 2023

I see.

So what then @ChristophWurst? Revert?

@dzatoah
Copy link
Contributor Author

dzatoah commented May 5, 2023

How would you estimate the effort? Is this doable for a Nextcloud newbie?

@ChristophWurst
Copy link
Member

This could be done with a repair step that reads any old value and write it under the new name. Repair steps look like this: https://github.com/nextcloud/server/blob/master/lib/private/Repair/OldGroupMembershipShares.php.

@ChristophWurst
Copy link
Member

The repair step would belong to the dav app, so maybe https://github.com/nextcloud/server/blob/master/apps/dav/lib/Migration/RemoveObjectProperties.php is better example. The step is then registered in

<repair-steps>
<post-migration>
<step>OCA\DAV\Migration\FixBirthdayCalendarComponent</step>
<step>OCA\DAV\Migration\RegenerateBirthdayCalendars</step>
<step>OCA\DAV\Migration\CalDAVRemoveEmptyValue</step>
<step>OCA\DAV\Migration\BuildCalendarSearchIndex</step>
<step>OCA\DAV\Migration\BuildSocialSearchIndex</step>
<step>OCA\DAV\Migration\RefreshWebcalJobRegistrar</step>
<step>OCA\DAV\Migration\RegisterBuildReminderIndexBackgroundJob</step>
<step>OCA\DAV\Migration\RemoveOrphanEventsAndContacts</step>
<step>OCA\DAV\Migration\RemoveClassifiedEventActivity</step>
<step>OCA\DAV\Migration\RemoveDeletedUsersCalendarSubscriptions</step>
<step>OCA\DAV\Migration\RemoveObjectProperties</step>
</post-migration>

This was referenced May 9, 2023
@ChristophWurst
Copy link
Member

@szaimen please revert this change

@szaimen
Copy link
Contributor

szaimen commented May 10, 2023

Done in #38177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants