Patch #33329

Improve watchers functionality to mark the users that are watching a non visible object and to not return watchers that cannot see the object

Added by Marius BALTEANU over 1 year ago. Updated 6 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues
Target version:4.2.0

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.

0002-Do-not-propose-watchers-that-are-not-allowed-to-see-.patch Magnifier (2.01 KB) Marius BALTEANU, 2020-04-20 14:48

0003-Show-an-warning-message-for-watchers-who-cannot-view.patch Magnifier (3.51 KB) Marius BALTEANU, 2020-04-20 14:48

0001-Fix-typo-in-find_objects_from_params.patch Magnifier (1.4 KB) Marius BALTEANU, 2020-04-20 14:48

invalid_watcher.png (89.6 KB) Marius BALTEANU, 2020-04-20 14:53


Related issues

Related to Redmine - Defect #22977: A project member has no access and gets no notification, ... New
Related to Redmine - Defect #5679: Watchers not cleaned/updated when deleting/moving watched... Closed 2010-06-12
Related to Redmine - Defect #11888: No e-mail notification for non-members who are watchers New
Related to Redmine - Patch #16133: Available watchers on new issue form include users who ca... Closed
Related to Redmine - Defect #35192: Watchers pop up window appears after a long time New

Associated revisions

Revision 20724
Added by Go MAEDA 6 months ago

Do not propose watchers that are not allowed to see the object (#33329).

Patch by Marius BALTEANU.

Revision 20725
Added by Go MAEDA 6 months ago

Show a warning message for watchers who cannot view the object (#33329).

Patch by Marius BALTEANU.

Revision 20726
Added by Go MAEDA 6 months ago

Update locales (#33329).

History

#1 Updated by Marius BALTEANU over 1 year ago

  • Related to Defect #22977: A project member has no access and gets no notification, when being a watcher of the issue added

#2 Updated by Marius BALTEANU over 1 year ago

  • Related to Defect #5679: Watchers not cleaned/updated when deleting/moving watched object added

#3 Updated by Go MAEDA over 1 year 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.

#4 Updated by Marius BALTEANU over 1 year ago

  • Assignee deleted (Marius BALTEANU)
  • 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.

#5 Updated by Go MAEDA 7 months ago

  • Related to Defect #11888: No e-mail notification for non-members who are watchers added

#6 Updated by Go MAEDA 6 months ago

  • Category set to Issues
  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patches. Thank you for improving Redmine.

#7 Updated by Go MAEDA 6 months ago

  • Related to Patch #16133: Available watchers on new issue form include users who cannot even view issues added

#8 Updated by Marius BALTEANU 3 months ago

  • Related to Defect #35192: Watchers pop up window appears after a long time added

Also available in: Atom PDF