Merge pull request #2400 from alphagov/fix-duplicate-user-bug-in-user…

…s-filter

Prevent duplicate rows in /users when filters applied

app/models/user.rb

@@ -88,7 +88,7 @@ class User < ApplicationRecord
   scope :not_setup_2sv, -> { not_exempt_from_2sv.does_not_have_2sv }
 
   scope :with_role, ->(role) { where(role:) }
-  scope :with_permission, ->(permission) { joins(:supported_permissions).merge(SupportedPermission.where(id: permission)) }
+  scope :with_permission, ->(permission) { joins(:supported_permissions).merge(SupportedPermission.unscoped.where(id: permission)).distinct }
   scope :with_organisation, ->(organisation) { where(organisation:) }
   scope :with_partially_matching_name, ->(name) { where(arel_table[:name].matches("%#{name}%")) }
   scope :with_partially_matching_email, ->(email) { where(arel_table[:email].matches("%#{email}%")) }

test/models/user_test.rb

@@ -1056,6 +1056,16 @@ def setup
       specified_permissions = [app1.signin_permission, app2.signin_permission]
       assert_equal [user1, user2], User.with_permission(specified_permissions.map(&:to_param))
     end
+
+    should "only return a user once even when they have two permissions for the same app" do
+      app = create(:application)
+
+      permission = create(:supported_permission, application: app)
+      create(:user, supported_permissions: [app.signin_permission, permission])
+
+      specified_permissions = [app.signin_permission, permission]
+      assert_equal 1, User.with_permission(specified_permissions.map(&:to_param)).count
+    end
   end
 
   context ".with_organisation" do

test/models/users_filter_test.rb

@@ -211,6 +211,17 @@ class UsersFilterTest < ActiveSupport::TestCase
 
       assert_equal %w[user1 user3], filter.users.map(&:name)
     end
+
+    should "return a user once if they have multiple selected permissions for the same app" do
+      app = create(:application, name: "App 1")
+      permission = create(:supported_permission, application: app, name: "Permission 1")
+
+      create(:user, name: "user", supported_permissions: [app.signin_permission, permission])
+
+      filter = UsersFilter.new(User.all, @current_user, permissions: [app.signin_permission, permission].map(&:to_param))
+
+      assert_equal %w[user], filter.users.map(&:name)
+    end
   end
 
   context "#permission_option_select_options" do