Skip to content

Skip instead of throwing on invalid DAV data migration #32466

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
skjnldsv merged 2 commits into from
Feb 23, 2024
Merged

Skip instead of throwing on invalid DAV data migration #32466

skjnldsv merged 2 commits into from
Feb 23, 2024

Conversation

@Pytal
Copy link
Member

@Pytal Pytal commented May 18, 2022

When encountering invalid data instead of failing we will skip it

@Pytal Pytal added bug 3. to review Waiting for reviews feature: dav feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals feature: user migration Related to User migration labels May 18, 2022
@Pytal Pytal added this to the Nextcloud 25 milestone May 18, 2022
@Pytal Pytal requested a review from a team May 18, 2022 01:34
@Pytal Pytal self-assigned this May 18, 2022
@Pytal Pytal requested review from PVince81, come-nc and icewind1991 and removed request for a team May 18, 2022 01:34
@Pytal
Copy link
Member Author

Pytal commented May 18, 2022

/backport to stable24

@Pytal Pytal requested a review from tcitworld May 18, 2022 17:08
tcitworld
tcitworld approved these changes May 19, 2022
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.

LGTM

@come-nc
Copy link
Contributor

come-nc commented May 19, 2022

This means partial import/partial export when there are errors in the data.
How is the user notified that the export is partial?
How is the user notified that the import is partial?

@Pytal
Copy link
Member Author

Pytal commented May 19, 2022

This means partial import/partial export when there are errors in the data. How is the user notified that the export is partial? How is the user notified that the import is partial?

When invoked via the CLI then the message is displayed though from the web UI there is no mechanism to communicate these messages

We could add a way for interface agnostic messages to be communicated or maybe something else if there's a better way, any further suggestions @come-nc?

This was referenced Aug 12, 2022
Merged
Merged
This was referenced Aug 24, 2022
Merged
Merged
This was referenced Sep 6, 2022
Merged
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
Merged
Closed
Merged
@blizzz blizzz mentioned this pull request May 17, 2023
Merged
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
This was referenced Nov 14, 2023
Merged
Merged
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
skjnldsv
skjnldsv reviewed Feb 23, 2024
View reviewed changes
@skjnldsv skjnldsv force-pushed the fix/skip-dav-migrator branch from 2e55cbf to a770fd8 Compare February 23, 2024 14:25
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 23, 2024
@skjnldsv skjnldsv enabled auto-merge February 23, 2024 14:31
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 23, 2024
View reviewed changes
apps/dav/lib/UserMigration/ContactsMigrator.php
$addressBookFilename,
$initialAddressBookUri,
$metadata,
$vCards,

Check notice

Code scanning / Psalm

ArgumentTypeCoercion

Argument 5 of OCA\DAV\UserMigration\ContactsMigrator::importAddressBook expects array<array-key, Sabre\VObject\Component\VCard>, but parent type array<array-key, Sabre\VObject\Component|Sabre\VObject\Component\VCard> provided
@skjnldsv skjnldsv disabled auto-merge February 23, 2024 20:28
@skjnldsv skjnldsv merged commit 3cc5079 into master Feb 23, 2024
@skjnldsv skjnldsv deleted the fix/skip-dav-migrator branch February 23, 2024 20:28
@backportbot backportbot bot mentioned this pull request Feb 23, 2024
Closed
2 tasks
@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@tcitworld tcitworld tcitworld approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@come-nc come-nc Awaiting requested review from come-nc

Assignees

@Pytal Pytal

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals feature: dav feature: user migration Related to User migration

Projects

None yet

Milestone

Nextcloud 29

Development

Successfully merging this pull request may close these issues.

6 participants

@Pytal @come-nc @tcitworld @skjnldsv @blizzz