From: Lucas Di Pentima <lucas@di-pentima.com.ar>
Date: Fri, 24 Jul 2020 14:47:16 +0000 (-0300)
Subject: 16470: Fixes test on user model.
X-Git-Tag: 2.1.0~132^2~13
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/498ac72a688f3ff44dc143dcbd248e3e8bc7cfe3

16470: Fixes test on user model.

Rails 5.1 deprecated the attr_changed? in favor of more explicit methods
because there was ambiguity when called from an 'after' or 'before' callback.
The test UsersTest#test_cannot_set_is_active_to_false_directly started failing
because User.setup is called from both types of callbacks, so its internal
checks weren't passing in some cases.

Also, avoids doing unnecessary queries to get the 'All users' group.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>
---

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index e1fd53e3de..778ad7d0bb 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -241,11 +241,8 @@ SELECT target_uuid, perm_level
                      name: 'can_login').destroy_all
 
     # delete "All users" group read permissions for this user
-    group = Group.where(name: 'All users').select do |g|
-      g[:uuid].match(/-f+$/)
-    end.first
     Link.where(tail_uuid: self.uuid,
-                     head_uuid: group[:uuid],
+                     head_uuid: all_users_group_uuid,
                      link_class: 'permission',
                      name: 'can_read').destroy_all
 
@@ -272,10 +269,6 @@ SELECT target_uuid, perm_level
        self.is_active_was &&
        !self.is_active
 
-      group = Group.where(name: 'All users').select do |g|
-        g[:uuid].match(/-f+$/)
-      end.first
-
       # When a user is set up, they are added to the "All users"
       # group.  A user that is part of the "All users" group is
       # allowed to self-activate.
@@ -290,7 +283,7 @@ SELECT target_uuid, perm_level
       # explaining the correct way to deactivate a user.
       #
       if Link.where(tail_uuid: self.uuid,
-                    head_uuid: group[:uuid],
+                    head_uuid: all_users_group_uuid,
                     link_class: 'permission',
                     name: 'can_read').any?
         errors.add :is_active, "cannot be set to false directly, use the 'Deactivate' button on Workbench, or the 'unsetup' API call"
@@ -711,11 +704,11 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
   # add the user to the 'All users' group
   def create_user_group_link
     return (Link.where(tail_uuid: self.uuid,
-                       head_uuid: all_users_group[:uuid],
+                       head_uuid: all_users_group_uuid,
                        link_class: 'permission',
                        name: 'can_read').first or
             Link.create(tail_uuid: self.uuid,
-                        head_uuid: all_users_group[:uuid],
+                        head_uuid: all_users_group_uuid,
                         link_class: 'permission',
                         name: 'can_read'))
   end
@@ -743,7 +736,8 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
   # Automatically setup if is_active flag turns on
   def setup_on_activate
     return if [system_user_uuid, anonymous_user_uuid].include?(self.uuid)
-    if is_active && (new_record? || saved_change_to_is_active?)
+    if is_active &&
+      (new_record? || saved_change_to_is_active? || will_save_change_to_is_active?)
       setup
     end
   end