Patch #38144

Refactoring: Use Group.visible instead of manual visibility check in GroupsController

Added by Holger Just about 1 month ago. Updated 29 days ago.

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

0%

Category:Groups
Target version:5.0.5

Description

The following patch adapts the find_group behavior in the GroupsController to directly rely on the Principal.visible scope rather than a manual check. The visible behavior should not change at all, the check is just at a more explicit area and thus is a little less likely to be forgotten with future changes.

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 69dd0d5920..a88c06391e 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -50,8 +50,6 @@ def index
   end

   def show
-    return render_404 unless @group.visible?
-
     respond_to do |format|
       format.html do
         render :layout => 'base'
@@ -149,7 +147,7 @@ def autocomplete_for_user
   private

   def find_group
-    @group = Group.find(params[:id])
+    @group = Group.visible.find(params[:id])
   rescue ActiveRecord::RecordNotFound
     render_404
   end

Related issues

Related to Redmine - Feature #12795: View group members by non-admin users Closed

Associated revisions

Revision 22023
Added by Go MAEDA about 1 month ago

Use Group.visible instead of manual visibility check in GroupsController (#38144).

Patch by Holger Just.

Revision 22024
Added by Go MAEDA 29 days ago

Merged r22023 from trunk to 5.0-stable (#38144).

History

#1 Updated by Holger Just about 1 month ago

  • Related to Feature #12795: View group members by non-admin users added

#2 Updated by Go MAEDA about 1 month ago

  • Target version set to 5.0.5

Setting the target version to 5.0.5.

#3 Updated by Go MAEDA about 1 month ago

  • Status changed from New to Resolved
  • Assignee set to Go MAEDA

Committed the fix. Thank you.

#4 Updated by Go MAEDA 29 days ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF