enh(contacts): show/hide addressbooks for all #38397

JohannesGGE · 2023-05-22T13:29:40Z

Fixes Add new option to hide an address book's contacts without disabling the address book contacts#479

Summary

TODO

...

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

JohannesGGE · 2023-05-22T14:00:21Z

Due to the different implementation of the recently contacted addressbook, the call for changing the enabled property can't be executed, and an "addressbook is immutable" is thrown in apps/contactsinteraction/lib/AddressBook.php::propPatch().

Other addressbooks call apps/dav/lib/CardDAV/AddressBook.php::propPatch()

As far as I can tell the only needed code is the DB update of apps/dav/lib/DAV/CustomPropertiesBackend.php::updateProperties() to have a working enable/disable for the recently contacted addressbook.

JohannesGGE · 2023-05-23T18:01:16Z

Recently contacted address book will be tackled in another issue.

ChristophWurst · 2023-05-24T06:01:31Z

apps/dav/lib/CardDAV/AddressBook.php

 	}

 	public function propPatch(PropPatch $propPatch) {
-		if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {


why is this dropped?

owner-principal seems to be set if the address book is shared, system or recently contacted. Own address books don't have this prop.

The change also surprised me.

Apparently, this was added before the CustomPropertiesBackend: 95e218b

If we throw forbidden here, the CustomPropertiesBackend::propPatch is not executed?

ChristophWurst · 2023-05-24T06:01:33Z

apps/dav/lib/CardDAV/AddressBook.php

-			]
+			],
+            [
+                'privilege' => '{DAV:}write-properties',


so this means users are now allowed to update properties of all address books, right? are there any property changes we might not want? e.g. rename.

just tested it with renaming, and it turns out the change of the name is not stored as a user prop but changes the real name, so the owner sees the change as well. I'll have a look in the code again to check where the distinction between the props is made.

Should be the special logic in https://github.com/nextcloud/server/blob/master/apps/dav/lib/DAV/CustomPropertiesBackend.php

@tcitworld As far as I understand it, the propPatch() in CustomPropertiesBackend.php is just called if the prop is a user prop. So the distinction is done somewhere before

JohannesGGE · 2023-06-02T10:59:14Z

The addressbook is now checked for sharing. If it is shared the props will be saved as userProps. So even if the addressbook is renamed by user that is not the owner (not possible in the contacts app yet), the owners addressbook name isn't changed. For the case the addressbook has a userProp name, this name is shown now.

tcitworld · 2023-06-02T12:21:02Z

apps/dav/lib/DAV/CustomPropertiesBackend.php

+		// substr of addressbooks/ => path is inside the CardDAV component
+		// three '/' => this a addressbook (no addressbook-home nor contact object)
+		if (substr($path, 0, 13) === 'addressbooks/' && substr_count($path, '/') === 3) {
+			$allRequestedProps = $propFind->getRequestedProperties();
+			$customPropertiesForShares = [
+				'{DAV:}displayname',
+			];
+
+			foreach ($customPropertiesForShares as $customPropertyForShares) {
+				if (in_array($customPropertyForShares, $allRequestedProps)) {
+					$requestedProps[] = $customPropertyForShares;
+				}
+			}
+		}
+


suggestion: I think there's a way to factorize this block with the block above in a single function which takes calendars and addressbooks specifics as parameters.

apps/dav/lib/DAV/CustomPropertiesBackend.php

Signed-off-by: Johannes Merkel <[email protected]>

kesselb · 2023-07-18T14:18:18Z

nitpick: enh(contacts) should be feat(contacts) according to https://www.conventionalcommits.org/en/v1.0.0/

kesselb

Well done 🎉

miaulalala · 2023-08-16T10:24:50Z

/backport to stable27

JohannesGGE added enhancement 2. developing Work in progress labels May 22, 2023

JohannesGGE self-assigned this May 22, 2023

JohannesGGE mentioned this pull request May 23, 2023

enh(addressbooks): add show/hide option for system and shared addressbooks nextcloud/contacts#3399

Merged

JohannesGGE added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 23, 2023

JohannesGGE marked this pull request as ready for review May 23, 2023 17:59

JohannesGGE requested review from ChristophWurst, miaulalala and tcitworld as code owners May 23, 2023 17:59

ChristophWurst reviewed May 24, 2023

View reviewed changes

ChristophWurst added feature: dav feature: caldav Related to CalDAV internals labels May 24, 2023

JohannesGGE added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 24, 2023

JohannesGGE marked this pull request as draft May 25, 2023 10:24

tcitworld reviewed Jun 2, 2023

View reviewed changes

JohannesGGE added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 13, 2023

JohannesGGE marked this pull request as ready for review June 13, 2023 08:49

JohannesGGE mentioned this pull request Jun 14, 2023

Add new option to hide an address book's contacts without disabling the address book nextcloud/contacts#479

Closed

JohannesGGE force-pushed the enh/contacts/hide-adressbook-option branch from 556aa6b to 60c9eb5 Compare June 23, 2023 15:03

JohannesGGE requested review from ChristophWurst, st3iny and tcitworld June 30, 2023 12:39

ChristophWurst reviewed Jul 6, 2023

View reviewed changes

apps/dav/lib/DAV/CustomPropertiesBackend.php Outdated Show resolved Hide resolved

apps/dav/lib/DAV/CustomPropertiesBackend.php Outdated Show resolved Hide resolved

JohannesGGE force-pushed the enh/contacts/hide-adressbook-option branch from 60c9eb5 to 7594644 Compare July 7, 2023 07:44

JohannesGGE force-pushed the enh/contacts/hide-adressbook-option branch from 7594644 to dbb174a Compare July 10, 2023 11:38

enh(contacts): show/hide addressbooks for all

bb80295

Signed-off-by: Johannes Merkel <[email protected]>

JohannesGGE force-pushed the enh/contacts/hide-adressbook-option branch from dbb174a to bb80295 Compare July 10, 2023 11:39

miaulalala approved these changes Jul 12, 2023

View reviewed changes

kesselb added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 18, 2023

kesselb approved these changes Jul 18, 2023

View reviewed changes

kesselb merged commit aa5dcb0 into master Jul 18, 2023

kesselb deleted the enh/contacts/hide-adressbook-option branch July 18, 2023 15:05

miaulalala mentioned this pull request Jul 21, 2023

fix(CardDAV): set owner-principal, displayname properties for SAB #39525

Merged

4 tasks

JohannesGGE mentioned this pull request Aug 16, 2023

[stable27] fix(CardDAV): set owner-principal, displayname properties for SAB #39900

Merged

backportbot-nextcloud bot added the backport-request label Aug 16, 2023

backportbot-nextcloud bot mentioned this pull request Aug 16, 2023

[stable27] enh(contacts): show/hide addressbooks for all #39904

Merged

backportbot-nextcloud bot removed the backport-request label Aug 16, 2023

JohannesGGE mentioned this pull request Sep 15, 2023

show/hide option for system missing nextcloud/contacts#3613

Closed

Uh oh!

enh(contacts): show/hide addressbooks for all #38397

enh(contacts): show/hide addressbooks for all #38397

Uh oh!

Conversation

JohannesGGE commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO

Checklist

Uh oh!

JohannesGGE commented May 22, 2023

Uh oh!

JohannesGGE commented May 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohannesGGE commented Jun 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kesselb commented Jul 18, 2023

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

miaulalala commented Aug 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JohannesGGE commented May 22, 2023 •

edited

Loading