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.

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

OCC command to delete calendars #26421

Merged
merged 1 commit into from
Jun 14, 2021
Merged

OCC command to delete calendars #26421

merged 1 commit into from
Jun 14, 2021

Conversation

mattian
Copy link
Contributor

@mattian mattian commented Apr 3, 2021 •
edited
Loading

Closes #25771.
Add occ command 'dav:delete-calendar' to delete user's calendars.

@rullzer rullzer added 3. to review Waiting for reviews feature: dav labels Apr 8, 2021
@rullzer rullzer added this to the Nextcloud 22 milestone Apr 8, 2021
tcitworld
tcitworld requested changes Apr 13, 2021
View reviewed changes
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Instead of a separate command to delete birthday calendars I would just go with a --birthday option for the delete-calendar command, which would set the calendar URI to BirthdayService::BIRTHDAY_CALENDAR_URI.

apps/dav/lib/Command/DeleteCalendar.php Outdated Show resolved Hide resolved
apps/dav/lib/Command/DeleteCalendar.php Outdated Show resolved Hide resolved
@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from 77e5a7d to aa4f375 Compare May 4, 2021 21:56
@mattian mattian changed the title OCC commands to delete calendars OCC command to delete calendars May 4, 2021
@mattian mattian requested a review from tcitworld May 12, 2021 20:34
tcitworld
tcitworld reviewed May 14, 2021
View reviewed changes
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Looks good to me now, but I recommend we wait for #26083 to be merged, then we can rebase on it and add a -f force parameter (which would be passed to CaldavBackend::deleteCalendar).

@mattian
Copy link
Contributor Author

mattian commented May 18, 2021

Ok, thank you

This was referenced May 20, 2021
Merged
Merged
@tcitworld
Copy link
Member

@mattian Please rebase on master and add the option to force delete calendars as discussed.

@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from aa4f375 to 3fe5a7c Compare May 31, 2021 20:12
@mattian mattian requested a review from tcitworld May 31, 2021 20:15
@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from 3fe5a7c to d643fc8 Compare May 31, 2021 21:22
@mattian
Copy link
Contributor Author

mattian commented May 31, 2021

done

@blizzz blizzz mentioned this pull request Jun 2, 2021
Merged
57 tasks
@ChristophWurst
Copy link
Member

@mattian please have a look at the inline annotations generated by psalm. a few types are not clear for the static analysis :)

blizzz
blizzz reviewed Jun 2, 2021
View reviewed changes
apps/dav/lib/Command/DeleteCalendar.php
InputInterface $input,
OutputInterface $output
): int {
$user = $input->getArgument('uid');
Copy link
Member

Choose a reason for hiding this comment

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

please cast to string to ensure this is string and silence the warnings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A cast makes psalm complain about a PossiblyInvalidCast from array<array-key, string> to string. Annotating $user as a string variable seems to silence the warnings

apps/dav/lib/Command/DeleteCalendar.php Outdated Show resolved Hide resolved
@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from d643fc8 to f8a3d2d Compare June 3, 2021 22:50
@mattian mattian requested a review from blizzz June 3, 2021 23:01
@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
Merged
59 tasks
@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from f8a3d2d to bb580f5 Compare June 10, 2021 19:09
tcitworld
tcitworld approved these changes Jun 11, 2021
View reviewed changes
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Lint is happy

ChristophWurst
ChristophWurst reviewed Jun 11, 2021
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Let's check if the duplicate logic of birthday calendars is necessary, the rest looks fine 👍

apps/dav/lib/Command/DeleteCalendar.php Outdated Show resolved Hide resolved
@mattian
occ: new command dav:delete-calendar
4850dae 
Add occ command 'dav:delete-calendar' to delete a user's calendar.

Signed-off-by: Mattia Narducci <[email protected]>
@mattian mattian force-pushed the enh/25771/occ-delete-calendar branch from bb580f5 to 4850dae Compare June 13, 2021 21:18
ChristophWurst
ChristophWurst approved these changes Jun 14, 2021
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good!

@ChristophWurst ChristophWurst merged commit dfbac05 into nextcloud:master Jun 14, 2021
@welcome
Copy link

welcome bot commented Jun 14, 2021

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@ChristophWurst
Copy link
Member

@mattian FYI your commit isn't associated with your github user. You might want to add the email address to your account so you are correctly shown as contributor :)

@blizzz blizzz mentioned this pull request Jun 16, 2021
Merged
45 tasks
@mattian mattian deleted the enh/25771/occ-delete-calendar branch August 22, 2021 17:53
mattian added a commit to mattian/nextcloud-documentation that referenced this pull request Aug 22, 2021
@mattian
admin_manual: add occ command dav:delete-calendar
8b3bb18 
Document occ command dav:delete-calendar introduced in Nextcloud 22.

See nextcloud/server#26421

Signed-off-by: Mattia Narducci <[email protected]>
@mattian mattian mentioned this pull request Aug 22, 2021
Merged
mattian added a commit to mattian/nextcloud-documentation that referenced this pull request Oct 12, 2021
@mattian
admin_manual: add occ command dav:delete-calendar
20b5a78 
Document occ command dav:delete-calendar introduced in Nextcloud 22.

See nextcloud/server#26421

Signed-off-by: Mattia Narducci <[email protected]>
backportbot-nextcloud bot pushed a commit to nextcloud/documentation that referenced this pull request Oct 13, 2021
@mattian @backportbot
admin_manual: add occ command dav:delete-calendar
097ed89 
Document occ command dav:delete-calendar introduced in Nextcloud 22.

See nextcloud/server#26421

Signed-off-by: Mattia Narducci <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@kesselb kesselb Awaiting requested review from kesselb

@miaulalala miaulalala Awaiting requested review from miaulalala

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

@rullzer rullzer Awaiting requested review from rullzer

@blizzz blizzz Awaiting requested review from blizzz

@tcitworld tcitworld Awaiting requested review from tcitworld

Assignees
No one assigned
Labels
3. to review Waiting for reviews feature: dav
Projects
None yet
Milestone
Nextcloud 22
Development

Successfully merging this pull request may close these issues.

Provide OCC command for deleting calendars
5 participants
@mattian @tcitworld @ChristophWurst @blizzz @rullzer