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

[Copy] New core setting : shareapi_only_share_with_group_members_exclude_gro… #43186

Closed
wants to merge 3 commits into from
Closed

[Copy] New core setting : shareapi_only_share_with_group_members_exclude_gro… #43186

wants to merge 3 commits into from

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Jan 29, 2024

Copy of #38173 to hopefully have it merged.

@artonge artonge added enhancement 3. to review Waiting for reviews feature: sharing javascript php Pull requests that update Php code pending documentation This pull request needs an associated documentation update labels Jan 29, 2024
@artonge artonge added this to the Nextcloud 29 milestone Jan 29, 2024
@artonge artonge requested review from come-nc and a team January 29, 2024 10:50
@artonge artonge self-assigned this Jan 29, 2024
@artonge artonge added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 29, 2024
@artonge artonge marked this pull request as ready for review January 29, 2024 10:51
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2024
View reviewed changes
apps/settings/lib/Settings/Admin/Sharing.php
@@ -63,6 +63,7 @@
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
$linksExcludedGroups = $this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', '');
$excludedPasswordGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
$onlyShareWithGroupMembersExcludeGroupList = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::getAppValue has been marked as deprecated
@come-nc
Copy link
Contributor

come-nc commented Jan 29, 2024 

There were 7 errors:

1) Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsWhenUserIsInExcludeGroups
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:219
/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:165
/home/runner/actions-runner/_work/server/server/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:249

2) Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsOnlyShareIfInTheSameGroup
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:219
/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:165
/home/runner/actions-runner/_work/server/server/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:320

3) Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsOnlyEnumerateIfInTheSameGroup
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:219
/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:165
/home/runner/actions-runner/_work/server/server/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:394

4) Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsOnlyEnumerateIfPhoneBookMatchWithOwnGroupsOnly
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:219
/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:165
/home/runner/actions-runner/_work/server/server/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:533

5) Tests\Contacts\ContactsMenu\ContactsStoreTest::testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnly
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:219
/home/runner/actions-runner/_work/server/server/lib/private/Contacts/ContactsMenu/ContactsStore.php:165
/home/runner/actions-runner/_work/server/server/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php:686

6) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetFormWithoutExcludedGroups
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/apps/settings/lib/Settings/Admin/Sharing.php:87
/home/runner/actions-runner/_work/server/server/apps/settings/tests/Settings/Admin/SharingTest.php:178

7) OCA\Settings\Tests\Settings\Admin\SharingTest::testGetFormWithExcludedGroups
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/home/runner/actions-runner/_work/server/server/apps/settings/lib/Settings/Admin/Sharing.php:87
/home/runner/actions-runner/_work/server/server/apps/settings/tests/Settings/Admin/SharingTest.php:270

@zak39
Copy link
Contributor

zak39 commented Jan 30, 2024

Hi @artonge :)

Why did you create a copy of PR #38173 ?

zak39 and others added 3 commits January 30, 2024 11:41
@zak39 @artonge
feat(php,vue): Create a new parameter
5293c8c 
Signed-off-by: Baptiste Fotia <fotia.baptiste@hotmail.com>
@zak39 @artonge
build(npm): Build assets
61ab517 
Signed-off-by: Baptiste Fotia <fotia.baptiste@hotmail.com>
@artonge
Fix tests
bd0092a 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the feature/37677/exclude-some-groups-from-sharing-with-users branch from 8d59add to bd0092a Compare January 30, 2024 10:41
@artonge
Copy link
Contributor Author

artonge commented Jan 30, 2024 •
edited
Loading

Why did you create a copy of PR #38173 ?

For some CI steps to get executed. But it looks like we just moved them from drone to GH actions this week. It is also easier for us to rebase and edit if the source branch is in our repo.

Can you try to rebase your PR ?

You will need to add the following changes to make tests greens:

You will also need to compile the front-end code again.

We can have a call tomorrow at 16h to do it together to make sure that your PR is merged this week.

@zak39
Copy link
Contributor

zak39 commented Jan 30, 2024

Why did you create a copy of PR #38173 ?

For some CI steps to get executed. But it looks like we just moved them from drone to GH actions this week. It is also easier for us to rebase and edit if the source branch is in our repo.

Can you try to rebase your PR ?

You will need to add the following changes to make tests greens:

* [apps/settings/tests/Settings/Admin/SharingTest.php](https://github.com/nextcloud/server/pull/43186/files#diff-e026f42c7a963e58e31fda9b288f348ba8cc988f537df0d88de84f400c68b854)

* [tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php](https://github.com/nextcloud/server/pull/43186/files#diff-c3ffe4d47c912fe61d37caa623f8a0030a30aa4ae1b7fea23153031e6e067cb0)

You will also need to compile the front-end code again.

We can have a call tomorrow at 16h to do it together to make sure that your PR is merged this week.

Sure, I'm going to rebase and build our PR in the early afternoon.
I will create an appointment with you 👍

Thanks 🙂

zak39 added a commit to arawa/server that referenced this pull request Jan 30, 2024
@zak39
test(php): Fix the unit tests
1ea9062 
I added our context in the unit tests following the advice of Louis.

Link : nextcloud#43186 (comment)

Signed-off-by: Baptiste Fotia <fotia.baptiste@hotmail.com>
zak39 added a commit to arawa/server that referenced this pull request Jan 30, 2024
@zak39
test(php): Fix the unit tests
abe6796 
I added our context in the unit tests following the advice of Louis.

Link : nextcloud#43186 (comment)

Signed-off-by: Baptiste Fotia <fotia.baptiste@hotmail.com>
@artonge artonge mentioned this pull request Jan 31, 2024
Merged
5 tasks
zak39 added a commit to arawa/server that referenced this pull request Jan 31, 2024
@zak39
test(php): Fix the unit tests
449d28e 
I added our context in the unit tests following the advice of Louis.

Link : nextcloud#43186 (comment)

Signed-off-by: Baptiste Fotia <fotia.baptiste@hotmail.com>
@artonge artonge closed this Jan 31, 2024
@smarinier
Copy link

Hi, the documentation PR is here : nextcloud/documentation#11537

@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Feb 23, 2024
@skjnldsv skjnldsv deleted the feature/37677/exclude-some-groups-from-sharing-with-users branch March 14, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@come-nc come-nc come-nc approved these changes

Assignees

@artonge artonge

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: sharing javascript pending documentation This pull request needs an associated documentation update php Pull requests that update Php code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@artonge @come-nc @zak39 @smarinier @susnux @skjnldsv