Extract SupportedPermission::SIGNIN_NAME

To remove duplication of the "signin" string.

app/models/doorkeeper/application.rb

@@ -11,13 +11,13 @@ class ::Doorkeeper::Application < ActiveRecord::Base
         lambda { |user|
           joins(supported_permissions: :user_application_permissions)
             .where("user_application_permissions.user_id" => user.id)
-            .where("supported_permissions.name" => "signin")
+            .where("supported_permissions.name" => SupportedPermission::SIGNIN_NAME)
             .where(retired: false)
         }
   scope :with_signin_delegatable,
         lambda {
           joins(:supported_permissions)
-            .where(supported_permissions: { name: "signin", delegatable: true })
+            .where(supported_permissions: { name: SupportedPermission::SIGNIN_NAME, delegatable: true })
         }
 
   after_create :create_signin_supported_permission
@@ -36,7 +36,7 @@ def supported_permission_strings(user = nil)
   end
 
   def signin_permission
-    supported_permissions.find_by(name: "signin")
+    supported_permissions.find_by(name: SupportedPermission::SIGNIN_NAME)
   end
 
   def sorted_supported_permissions_grantable_from_ui
@@ -78,7 +78,7 @@ def substituted_uri(uri)
   end
 
   def create_signin_supported_permission
-    supported_permissions.create!(name: "signin", delegatable: true)
+    supported_permissions.create!(name: SupportedPermission::SIGNIN_NAME, delegatable: true)
   end
 
   def create_user_update_supported_permission

app/models/supported_permission.rb

@@ -1,4 +1,6 @@
 class SupportedPermission < ApplicationRecord
+  SIGNIN_NAME = "signin".freeze
+
   belongs_to :application, class_name: "Doorkeeper::Application"
   has_many :user_application_permissions, dependent: :destroy, inverse_of: :supported_permission
 
@@ -11,15 +13,15 @@ class SupportedPermission < ApplicationRecord
   scope :default, -> { where(default: true) }
 
   def signin?
-    name.try(:downcase) == "signin"
+    name.try(:downcase) == SIGNIN_NAME
   end
 
 private
 
   def signin_permission_name_not_changed
     return if new_record? || !name_changed?
 
-    if name_change.first.casecmp("signin").zero?
+    if name_change.first.casecmp(SIGNIN_NAME).zero?
       errors.add(:name, "of permission #{name_change.first} can't be changed")
     end
   end

app/models/user.rb

@@ -168,7 +168,7 @@ def revoke_all_authorisations
   end
 
   def grant_application_signin_permission(application)
-    grant_application_permission(application, "signin")
+    grant_application_permission(application, SupportedPermission::SIGNIN_NAME)
   end
 
   def grant_application_permission(application, supported_permission_name)

app/views/shared/_user_permissions.html.erb

@@ -31,19 +31,19 @@
             # API Users will always have a "signin" permission for apps for which they have access token.
             # The hidden field ensures it is not lost.
           %>
-          <%= hidden_field_tag supported_permission_field_name, application.signin_permission.id, id: "#{supported_permission_field_prefix}_signin" %>
+          <%= hidden_field_tag supported_permission_field_name, application.signin_permission.id, id: "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}" %>
         <% else %>
           <td>
-            <%= label_tag "#{supported_permission_field_prefix}_signin", "Has access to #{application.name}?", class: "rm" %>
+            <%= label_tag "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}", "Has access to #{application.name}?", class: "rm" %>
             <%= check_box_tag supported_permission_field_name, application.signin_permission.id, user_object.has_access_to?(application),
-                              id: "#{supported_permission_field_prefix}_signin" %>
+                              id: "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}" %>
           </td>
         <% end %>
         <td>
           <%= label_tag "#{supported_permission_field_prefix}_ids", "Permissions for #{application.name}", class: "rm" %>
           <% supported_permissions_options = application.supported_permissions.grantable_from_ui
                                                .inject({}) {|h, per| h.merge(per.name => per.id) }
-             supported_permissions_options.delete('signin') %>
+             supported_permissions_options.delete(SupportedPermission::SIGNIN_NAME) %>
           <%= select_tag supported_permission_field_name,
                           options_for_select(supported_permissions_options,
                             user_object.permission_ids_for(application) - [application.signin_permission.id]),

app/views/users/_app_permissions.html.erb

@@ -22,11 +22,11 @@
         </td>
         <% unless user_object.api_user? %>
           <td>
-            <%= permission_names.include?('signin') ? 'Yes' : 'No' %>
+            <%= permission_names.include?(SupportedPermission::SIGNIN_NAME) ? 'Yes' : 'No' %>
           </td>
         <% end %>
         <td>
-          <%= permission_names.reject{|p| p == 'signin'}.to_sentence %>
+          <%= permission_names.reject{|p| p == SupportedPermission::SIGNIN_NAME}.to_sentence %>
         </td>
       </tr>
     <% end %>

lib/numbers/metrics.rb

@@ -65,7 +65,7 @@ def to_a
   private
 
     def has_signin_permissions?(permission)
-      permission.permissions.include?("signin")
+      permission.permissions.include?(SupportedPermission::SIGNIN_NAME)
     end
 
     def count_values(map)

test/controllers/authorisations_controller_test.rb

@@ -62,7 +62,7 @@ class AuthorisationsControllerTest < ActionController::TestCase
 
         post :create, params: { api_user_id: @api_user.id, doorkeeper_access_token: { application_id: @application.id } }
 
-        assert_equal %w[signin], @api_user.permissions_for(@application)
+        assert_equal [SupportedPermission::SIGNIN_NAME], @api_user.permissions_for(@application)
       end
     end
   end

test/controllers/root_controller_test.rb

@@ -26,7 +26,7 @@ def setup
   test "Your applications should include apps you have permission to signin to" do
     exclusive_app = create(:application, name: "Exclusive app")
     everybody_app = create(:application, name: "Everybody app")
-    user = create(:user, with_permissions: { exclusive_app => [], everybody_app => %w[signin] })
+    user = create(:user, with_permissions: { exclusive_app => [], everybody_app => [SupportedPermission::SIGNIN_NAME] })
 
     sign_in user
 

test/controllers/users_controller_test.rb

@@ -200,7 +200,7 @@ def change_user_password(user_factory, new_password)
       @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
       get :show, params: { client_id: @application.uid, format: :json }
       json = JSON.parse(response.body)
-      assert_equal(%w[signin], json["user"]["permissions"])
+      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
     end
 
     should "fetching json profile should include only permissions for the relevant app" do
@@ -212,7 +212,7 @@ def change_user_password(user_factory, new_password)
       @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
       get :show, params: { client_id: @application.uid, format: :json }
       json = JSON.parse(response.body)
-      assert_equal(%w[signin], json["user"]["permissions"])
+      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
     end
 
     should "fetching json profile should update last_synced_at for the relevant app" do
@@ -469,10 +469,10 @@ def change_user_password(user_factory, new_password)
       end
 
       should "can give permissions to all applications" do
-        delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-        non_delegatable_app = create(:application, with_supported_permissions: %w[signin])
-        delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-        non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin])
+        delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+        non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+        delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+        non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
 
         @user.grant_application_signin_permission(delegatable_app)
         @user.grant_application_signin_permission(non_delegatable_app)
@@ -507,10 +507,10 @@ def change_user_password(user_factory, new_password)
         end
 
         should "be able to give permissions only to applications they themselves have access to and that also have delegatable signin permissions" do
-          delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-          non_delegatable_app = create(:application, with_supported_permissions: %w[signin])
-          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin])
+          delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
 
           organisation_admin = create(:organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app])
 
@@ -533,10 +533,10 @@ def change_user_password(user_factory, new_password)
         end
 
         should "be able to see all permissions to applications for a user" do
-          delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor])
-          non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"])
-          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"])
-          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"])
+          delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"])
+          non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"])
+          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"])
+          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"])
 
           organisation_admin = create(:organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app])
 
@@ -546,9 +546,9 @@ def change_user_password(user_factory, new_password)
             :user_in_organisation,
             organisation: organisation_admin.organisation,
             with_permissions: { delegatable_app => %w[Editor],
-                                non_delegatable_app => ["signin", "GDS Admin"],
-                                delegatable_no_access_to_app => ["signin", "GDS Editor"],
-                                non_delegatable_no_access_to_app => ["signin", "Import CSVs"] },
+                                non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"],
+                                delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"],
+                                non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] },
           )
 
           get :edit, params: { id: user.id }
@@ -590,10 +590,10 @@ def change_user_password(user_factory, new_password)
         end
 
         should "be able to give permissions only to applications they themselves have access to and that also have delegatable signin permissions" do
-          delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-          non_delegatable_app = create(:application, with_supported_permissions: %w[signin])
-          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin])
-          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin])
+          delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME])
+          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
 
           super_org_admin = create(:super_organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app])
 
@@ -616,10 +616,10 @@ def change_user_password(user_factory, new_password)
         end
 
         should "be able to see all permissions to applications for a user" do
-          delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor])
-          non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"])
-          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"])
-          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"])
+          delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"])
+          non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"])
+          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"])
+          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"])
 
           super_org_admin = create(:super_organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app])
 
@@ -629,9 +629,9 @@ def change_user_password(user_factory, new_password)
             :user_in_organisation,
             organisation: super_org_admin.organisation,
             with_permissions: { delegatable_app => %w[Editor],
-                                non_delegatable_app => ["signin", "GDS Admin"],
-                                delegatable_no_access_to_app => ["signin", "GDS Editor"],
-                                non_delegatable_no_access_to_app => ["signin", "Import CSVs"] },
+                                non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"],
+                                delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"],
+                                non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] },
           )
 
           get :edit, params: { id: user.id }
@@ -660,10 +660,10 @@ def change_user_password(user_factory, new_password)
 
       context "superadmin" do
         should "not be able to see all permissions to applications for a user" do
-          delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor])
-          non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"])
-          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"])
-          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"])
+          delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"])
+          non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"])
+          delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"])
+          non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"])
 
           superadmin = create(:superadmin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app])
 
@@ -673,9 +673,9 @@ def change_user_password(user_factory, new_password)
             :user_in_organisation,
             organisation: superadmin.organisation,
             with_permissions: { delegatable_app => %w[Editor],
-                                non_delegatable_app => ["signin", "GDS Admin"],
-                                delegatable_no_access_to_app => ["signin", "GDS Editor"],
-                                non_delegatable_no_access_to_app => ["signin", "Import CSVs"] },
+                                non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"],
+                                delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"],
+                                non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] },
           )
 
           get :edit, params: { id: user.id }

test/factories/oauth_applications.rb

@@ -16,19 +16,19 @@
       evaluator.with_supported_permissions.each do |permission_name|
         # we create signin in an after_create on application.
         # this line takes care of tests creating signin in order to look complete or modify delegatable on it.
-        app.signin_permission.update(delegatable: false) && next if permission_name == "signin"
+        app.signin_permission.update(delegatable: false) && next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:supported_permission, application_id: app.id, name: permission_name)
       end
 
       evaluator.with_supported_permissions_not_grantable_from_ui.each do |permission_name|
-        next if permission_name == "signin"
+        next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:supported_permission, application_id: app.id, name: permission_name, grantable_from_ui: false)
       end
 
       evaluator.with_delegatable_supported_permissions.each do |permission_name|
-        next if permission_name == "signin"
+        next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:delegatable_supported_permission, application_id: app.id, name: permission_name)
       end

test/integration/api_authentication_test.rb

@@ -11,7 +11,7 @@ def access_user_endpoint(token = nil, params = {})
 
   setup do
     @app1 = create(:application, name: "MyApp", with_supported_permissions: %w[write])
-    @user = create(:user, with_permissions: { @app1 => %w[signin write] })
+    @user = create(:user, with_permissions: { @app1 => [SupportedPermission::SIGNIN_NAME, "write"] })
     @user.authorisations.create!(application_id: @app1.id)
   end
 

test/integration/batch_inviting_users_test.rb

@@ -77,7 +77,7 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest
 
   should "ensure that batch invited users get default permissions even when not checked in UI" do
     create(:supported_permission, application: @application, name: "reader", default: true)
-    support_app = create(:application, name: "support", with_supported_permissions: %w[signin])
+    support_app = create(:application, name: "support", with_supported_permissions: [SupportedPermission::SIGNIN_NAME])
     support_app.signin_permission.update!(default: true)
     user = create(:admin_user)