Patch #33329
closed
Improve watchers functionality to mark the users that are watching a non visible object and to not return watchers that cannot see the object
Description
The scope of this issue is to decrease the confusion created by the actual watchers implementation which allows you to add to an object (issue, wiki, news) a watcher who doesn't have permissions to view that object. There are multiple open issues generated by this issue, but I opted to create a new issue, clean, to discuss some proposal that I have in plan. At the end, depending on what it's implemented, we close those issues.
I made three patches:
1. 0001-Fix-typo-in-find_objects_from_params.patch
It just fix a typo in the code.
2. 0002-Do-not-propose-watchers-that-are-not-allowed-to-see-.patch
The patch changes the behaviour of method users_for_new_watcher to reject users who doesn't have the permissions to view the object. This change applies only when the search is made for one object (eg: issue), not for multiple objects (add bulk watchers).
3. 0003-Show-an-warning-message-for-watchers-who-cannot-view
Because the second patch doesn't cover all the cases and it's difficult to cover all the cases, I've added in the UI an warning next to an invalid watcher with the following warning message: "Invalid watcher: User will not receive any notifications because it does not have access to view this object.". Any feedback on the message is welcome.
What else I have in mind:
4. Limit the number of results returned by the auto complete method to a maximum 20 or 30 objects in order to avoid performance issues (we need to iterate through each object to check the visibility). Right now, when a term is used in the search, the autocomplete returns all the results found, else returns the first 100 results. It's hard to believe that an user prefer to scroll down in the list of returned watchers instead of typing a more complete search term.
5. Return an error or an warning message when a watcher is added to an object not visible.
Files
Related issues
Updated by Marius BĂLTEANU over 4 years ago
- Related to Defect #22977: A project member has no access and gets no notification, when being a watcher of the issue added
Updated by Marius BĂLTEANU over 4 years ago
- Related to Defect #5679: Watchers not cleaned/updated when deleting/moving watched object added
Updated by Go MAEDA over 4 years ago
Looks good to me.
A similar patch #16133 was rejected 5 years ago due to performance concerns when a project has a lot of members, but I think 0002-Do-not-propose-watchers-that-are-not-allowed-to-see-.patch does not introduce the performance problem because watchable_object.visible? is only called 200 times, only when the user open Add watchers dialog.
Updated by Marius BĂLTEANU over 4 years ago
- Assignee deleted (
Marius BĂLTEANU) - Target version changed from Candidate for next major release to 4.2.0
Go MAEDA wrote:
Looks good to me.
A similar patch #16133 was rejected 5 years ago due to performance concerns when a project has a lot of members, but I think 0002-Do-not-propose-watchers-that-are-not-allowed-to-see-.patch does not introduce the performance problem because
watchable_object.visible?is only called 200 times, only when the user open Add watchers dialog.
I'm adding this to 4.2.0 in order to discuss and find some solutions to fix the current annoying behaviour. We can limit the number of calls made by watchable_object.visible? by limiting the number of results returned from the database (proposal nr#4). Regarding patch nr#3, if the overall idea is accepted, we can load the watchers async always or only when there are more than x watchers. I'm sure that we can find good solutions without performance degradation.
Updated by Go MAEDA about 4 years ago
- Related to Defect #11888: No e-mail notification for non-members who are watchers added
Updated by Go MAEDA almost 4 years ago
- Category set to Issues
- Status changed from New to Closed
- Assignee set to Go MAEDA
Committed the patches. Thank you for improving Redmine.
Updated by Go MAEDA almost 4 years ago
- Related to Patch #16133: Available watchers on new issue form include users who cannot even view issues added
Updated by Marius BĂLTEANU over 3 years ago
- Related to Defect #35192: Watchers pop up window appears after a long time added
Updated by Holger Just almost 3 years ago
- Related to Defect #36549: Issues with watchers and restricted tickets added
Updated by Holger Just over 2 years ago
- Related to Defect #37224: Add watchers to issue added
Updated by Jenda Benda about 1 month ago
Hello, everyone,
thanks for your work.
Related to these changes, I believe, having ability to add us user as a watcher and to get a message later on he/she can see the issue is little bit weird...
Also adding rights to the watcher for the issue means currently (if I'm not mistaken) to add him ability to see all the issues within whole project, which is not wanted in most cases. When someone can't see all the issues it's usually ofr some reason. And when adding as a watcher of one shouldn't require permission for all.
I believe we need something like this (soft read only permission for watched issues):
https://github.com/maxrossello/redmine_extended_watchers
Thank you so much
Kind regards
Jan
Updated by Holger Just about 1 month ago
Jenda Benda wrote in #note-11:
Related to these changes, I believe, having ability to add us user as a watcher and to get a message later on he/she can see the issue is little bit weird...
In an unchanged Redmine, you can only add a watcher to an issue if the user can already see the issue. It is however possible that later on, the user loses the right to see the issue. In that case, they still remain a watcher but will not receive any notifications for the issue nor will they be able to see it. For other users, the watcher will then be rendered with the "invalid watcher" message.
Also adding rights to the watcher for the issue means currently (if I'm not mistaken) to add him ability to see all the issues within whole project, which is not wanted in most cases.
That is not correct. In a default Redmine, adding a watcher does not add any additional permissions and thus does not allow the watcher to see an issue they can not see otherwise. There is however a feature request to update the watchers functionality to grant watchers read-only permissions to issues they watch, see #8488.
Here (and in all plugins and related implementations I'm aware, including on Planio), adding a watcher to an issue will only grant the watcher additional read access to this single issue, not to all issues in the project.
When someone can't see all the issues it's usually ofr some reason. And when adding as a watcher of one shouldn't require permission for all.
Yes, that is the behavior of Redmine.
I believe we need something like this (soft read only permission for watched issues):
This plugin implements a similar functionality to what is described in #8488, #7412, #23546 and several other similar issues. Initially this change was rejected by Jean-Philippe Lang. See #7412#note-13
Updated by Jenda Benda about 1 month ago
Hello, thanks for the reply and all the information.
I'm not sure why, but in our redmine instance we can add "watchers" role to the users that can't see the issue, but are members of the same project. The instalation is 2 week fresh, where we have just "additionals" plugin installed. This is probably the only thing that might change the default behavior, but I doubt it is the case... Any idea how's that possible then?
That's the reason why we are getting into the weird situation, where anyone can assign as a watcher anyone from the same project(customer), but then gets the warning message about the user not being able to see the issue.
That is not correct. In a default Redmine, adding a watcher does not add any additional permissions and thus does not allow the watcher to see an issue they can not see otherwise. There is however a feature request to update the watchers functionality to grant watchers read-only permissions to issues they watch, see #8488.
I'm sorry for bad explanation/wording... I meant that currently if we would like to enable watchers to be added to the certain issues in their project....we have to allow them to see all the issues of the project manually first.
Is there any other way how to assign an issue to the users of the same project who can't see all the issues within the project?
The thing is we were used to (e.g. in Jira) to do that. But at the same time we do not want users to see all issues by default (just in case anyone would like to assign them as watchers)...
thank you so much
Kind regards
Jan
Updated by Holger Just about 1 month ago
Please move this discussion over to the forums. This unrelated and closed issue is not the right place for this.
Updated by Jenda Benda about 1 month ago
OK, I will and thank you so much.
just a quick note...following permissions setting with "add watcher" permission allow users to add watcher to issues thay can't see-(
I'd assume, that in case we "assign" an issue to a user in the form of "watcher" it should be considered as an issue the "watching user" should be able to see also. Not to see just issues directly assigned as a resolver/assignee, otherwise this possibility doesn't make much sense and user will end-up with error messaage (This message will be shown only after assigning user as a watcher).
Btw. I've just found out that what I stated is valid for newly created issues only - in the porcess of creating new issue, a watcher can be added despite the fact he/she can't see the issue. When you try to do the same for an existing issue, it won't show you users who can't see the issue.
thx
Kind regards
j.