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

Exclude groups for sharing #49217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+21 −0
Open

Exclude groups for sharing #49217

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03e4ba8
Exclude groups for sharing
tomwezepoel Nov 12, 2024
fa1a5ae
Merge branch 'master' into 49198-exclude_groups_for_sharing
tomwezepoel Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions apps/files_sharing/lib/Controller/ShareesAPIController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,27 @@ public function search(string $search = '', ?string $itemType = null, int $page
$result['exact'] = array_merge($this->result['exact'], $result['exact']);
}
$this->result = array_merge($this->result, $result);

// Get a list of groups to exclude from search results
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', true);
Comment on lines +201 to +202
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Get a list of groups to exclude from search results
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', true);
// Get a list of groups to exclude from search results
$sharingDisabledGroups = $this->config->getSystemValue('sharing.disabled_groups', true);

This name is too generic for a system option and blacklist should be avoided (it's better and clearer to use blocklist).

Comment on lines +201 to +202
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Get a list of groups to exclude from search results
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', true);
// Get a list of groups to exclude from search results
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', []);

The default value needs to be an empty array, as otherwise the later code will fail if the config is not set.

Copy link
Contributor

@nfebe nfebe Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', true);
$blacklisted_groups = $this->config->getSystemValue('blacklisted_groups', true);
if (!is_array($blacklisted_groups)) {
$blacklisted_groups = [];
}

Graceful handling if non array is passed? This can be needless if the source if SURE to provide arrays (when probably not configurable)

Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine to assume we get an array, but the default value needs to be an array like I already suggested above.


// Check and filter normal 'groups' array at the top level and the nested 'exact' level
if (isset($this->result['groups'])) {
$this->result['groups'] = array_filter($this->result['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['groups'] = array_values($this->result['groups']);
Comment on lines +206 to +210
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$this->result['groups'] = array_filter($this->result['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['groups'] = array_values($this->result['groups']);
$this->result['groups'] = array_values(array_filter($this->result['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
}));

}

if (isset($this->result['exact']['groups'])) {
$this->result['exact']['groups'] = array_filter($this->result['exact']['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['exact']['groups'] = array_values($this->result['exact']['groups']);
Comment on lines +214 to +218
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$this->result['exact']['groups'] = array_filter($this->result['exact']['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['exact']['groups'] = array_values($this->result['exact']['groups']);
$this->result['exact']['groups'] = array_values(array_filter($this->result['exact']['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
}));

}
Comment on lines +205 to +219
Copy link
Contributor

@nfebe nfebe Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
if (isset($this->result['groups'])) {
$this->result['groups'] = array_filter($this->result['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['groups'] = array_values($this->result['groups']);
}
if (isset($this->result['exact']['groups'])) {
$this->result['exact']['groups'] = array_filter($this->result['exact']['groups'], function($group) use ($blacklisted_groups) {
return !in_array($group['label'], $blacklisted_groups);
});
// Reindex array to maintain sequential numeric keys
$this->result['exact']['groups'] = array_values($this->result['exact']['groups']);
}
$filterBlacklistedGroups = function ($groups) use ($blacklisted_groups) {
return array_values(array_filter($groups, function ($group) use ($blacklisted_groups) {
return isset($group['label']) && !in_array($group['label'], $blacklisted_groups);
}));
};
if (isset($this->result['groups'])) {
$this->result['groups'] = $filterBlacklistedGroups($this->result['groups']);
}
if (isset($this->result['exact']['groups'])) {
$this->result['exact']['groups'] = $filterBlacklistedGroups($this->result['exact']['groups']);
}

Since the filter logic is repeated, you can use an inline closure..... or define a function outside this method.


$response = new DataResponse($this->result);

if ($hasMoreResults) {
Expand Down