Feature : targeted private saved searches

[ArthurMinfotel]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets

Add option to choose targets for private saved searches.
Modify CommonDBVisible to make it possible to restrict the types of target available, and to choose the right used to determine the availability of user type targets.
For a saved search, a target can either be a user or a group.
User and group dropdown take into account entity of the item when loading available targets.

The class used for the user target might need to be changed (SavedSearch_User is already used so I couldn't follow the naming convention set by the other CommonDBVisible items, so I created SavedSearch_UserTarget for now).
Also the new method getVisibilityRight could be used to dynamically get the right for the itemtype associated with the saved search (to only be able to targets users with the read right on the itemtype).

Example :
Target tab for SavedSearch :

Corresponding record in DB :

Available saved searches for the itemtype for users 2 without any target set :

User 2 added as a target :

Available saved searches on the itemtype for user 2 after being added added as a target :

Type of targets available set through the static variable $types :

Restrictions on groups available as targets based on the item's entity (for the example entity set as 1 instead of root) :
List of entities :

List of groups with entity restrictions :

Visibility for groups :

Subvisibility :

[cedric-anne]

@ArthurMinfotel

Could you add some screenshots to your PR description?

[ArthurMinfotel]

@ArthurMinfotel

Could you add some screenshots to your PR description?

I've edited it to add several screenshoots, showing an example and the added functionnalities.

[cedric-anne]

We discussed about it with @orthagh and we think that the is_private flag should be removed from the database. Indeed, having both a public/private management + a entity/groups/users visibility management is redundant and can be a bit complex to handle for the end user.

At creation, if the saved search is created as public, then it a visibility on the root entity + recursive should be automatically created. If it is created as private, the lack of visibility filters should be sufficient to make it private.

It would require to add a migration to add a new visibility filter to all existing public searches.

[ArthurMinfotel]

If everything is OK from a pratical point of view, I will start working on updating the current test functions and adding a new one for the targets.

[ArthurMinfotel]

Tests are updated.
Things that are tested are :

1. Automatically adding the saved search's entity as target if input contains is_private with a falsy value.
2. For each type of target, visibility of a saved search to a user other than the creator when using SavedSearch::getMine().

[AdrienClairembault]

It seems like there are still some small lint issues.

For the sonar cloud report, it does not like that you are created an item using a class supplied by the browser:

This has created security issues in the past as malicious users can supply some unwanted class names.
You can fix it by calling getItemForItemtype instead, which will make sure the class is at least a CommonGLPI item which should be safe.

[ArthurMinfotel]

I've updated the controller to fix the issue with the class instance.

For the lint, I believe those are not fixable :

The first and second are called in getTabNameForItem and displayTabContentForItem, and so lint doesn't recognise the methods from the class CommonDBVisible, which are called when $item is an instance of SavedSearch , since its parameter $item is typed as CommonGLPI.
As the third one, it's a right check, so I don't even know what the cause might be.

[AdrienClairembault]

I've added suggestions that should fix the typing issues.

For the "always true" issue, this is because the condition exist twice:

So when you reach the second condition, it will always be true because if the first one was false then the function would have returned a value already.

I'll let you review to see which one you need to keep.

[ArthurMinfotel]

I added the suggested fix. I will remember those for future developments.

[AdrienClairembault]

Ok for me besides the right check.