Merge pull request #2590 from alphagov/extract-user-web-user-predicate

Extract `User#web_user?` predicate method
alphagov · Dec 12, 2023 · 7601f8b · 7601f8b
2 parents 2a8b4cf + c006e59
commit 7601f8b
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 7 deletions.
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -285,7 +285,7 @@ def lock_access!(opts = {})
   def status
     return USER_STATUS_SUSPENDED if suspended?
 
-    if !api_user? && invited_but_not_yet_accepted?
+    if web_user? && invited_but_not_yet_accepted?
       return USER_STATUS_INVITED
     end
 
@@ -419,6 +419,10 @@ def organisation_name
     organisation.present? ? organisation.name : Organisation::NONE
   end
 
+  def web_user?
+    !api_user?
+  end
+
 private
 
   def two_step_mandated_changed?

diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
@@ -48,7 +48,7 @@ def exempt_from_two_step_verification?
     current_user.belongs_to_gds? &&
       current_user.govuk_admin? &&
       record.normal? &&
-      !record.api_user
+      record.web_user?
   end
 
 private

diff --git a/app/views/shared/_user_permissions.html.erb b/app/views/shared/_user_permissions.html.erb
@@ -2,10 +2,10 @@
   <thead>
     <tr class="table-header">
       <th>Application</th>
-      <% unless user_object.api_user? %>
+      <% if user_object.web_user? %>
         <th>Has access?</th>
       <% end %>
-      <th><%= "Other" unless user_object.api_user? %> Permissions</th>
+      <th><%= "Other" if user_object.web_user? %> Permissions</th>
       <% if user_object.persisted? %>
         <th>Last synced at</th>
       <% end %>

diff --git a/app/views/users/_app_permissions.html.erb b/app/views/users/_app_permissions.html.erb
@@ -2,10 +2,10 @@
   <thead>
     <tr class="table-header">
       <th>Application</th>
-      <% unless user_object.api_user? %>
+      <% if user_object.web_user? %>
           <th>Has access?</th>
       <% end %>
-      <th><%= "Other" unless user_object.api_user? %> Permissions</th>
+      <th><%= "Other" if user_object.web_user? %> Permissions</th>
     </tr>
   </thead>
   <tbody>
@@ -16,7 +16,7 @@
         <td>
           <%= application.name %>
         </td>
-        <% unless user_object.api_user? %>
+        <% if user_object.web_user? %>
           <td>
             <%= permission_names.include?(SupportedPermission::SIGNIN_NAME) ? 'Yes' : 'No' %>
           </td>

diff --git a/test/models/user_test.rb b/test/models/user_test.rb
@@ -1276,6 +1276,16 @@ def setup
     end
   end
 
+  context "#web_user?" do
+    should "return true for non-API user" do
+      assert build(:user).web_user?
+    end
+
+    should "return false for API user" do
+      assert_not build(:api_user).web_user?
+    end
+  end
+
   def authenticate_access(user, app)
     Doorkeeper::AccessToken.create!(resource_owner_id: user.id, application_id: app.id)
   end