Redmine

Attached is a patch against current trunk which fixes this behavior. The patch was extracted from Planio.

The patch is more exhaustive than the previous ones by Salman:

• It avoid having to load all groups as AR records
• It also patches the instance watched_by? method to match the behavior of the scope.
• It adds a test :)

I have also updated the parameter name of the watched_by scope to make it clear that it accepts a principal (either a user or group), rather than a numeric ID. Previously, the method accepted both types. In all call-sites within Redmine however, only a user or group object is ever passed here, never a numeric ID.

With that being said, the way the scope is implemented, it will perform some SQL queries already when building the scope to get the groups of the passed user. This might be undesirable, e.g. if there are updates here which might affect the final query between building the scope and executing it. An alternative to the scope in the attached patch would thus be the following scope which builds a fully self-contained query where all data is only resolved during its execution:

scope :watched_by, lambda { |user_id|
  # Basic case: return objects watched by the queried principal
  scope = where("#{Watcher.table_name}.user_id = ?", user_id)

  # If the principal is not a group (i.e. either a User or the id of
  # either a User or Group)...
  unless user_id.is_a?(Group)
    # ... we make sure that the given id belongs to an actual User...
    if user_id.is_a?(Integer)
      is_user = where(User.where(id: user_id).arel.exists)
    else
      is_user = self
    end

    # ... and finally allow objects which are either watched by the
    # user (base case above) or by any group the user is a member of
    scope = scope.or(
      is_user.
      where("#{Watcher.table_name}.user_id IN (?)", Group.having_user(user_id).select(:id))
    )
  end

  # Finally, we have to join the watchers table at the end to simplify
  # the or condition above. The final SQL is the same in any case.
  scope.joins(:watchers)
}

This scope is functionally equivalent to the version in the patch (with the addition that we do support passed Integer arguments for the id of a user or group here). It will not make any queries on its own, thus avoiding additional roundtrips to the database. With that however, it will also not use any cached data (such as a previously cached principal.group_ids list).

I think it is safe to deliver this fix in 5.0.2.

Patch committed, thanks!