Merge pull request #11606 from CartoDB/11407-reorder_eumapi_deletion

Reorder user deletion so we delete from box and then Central

NEWS.md

@@ -85,6 +85,7 @@ Development
 * Fix layer loading at embeds (#11554)
 * Restrict login from organization pages to organization users, and redirect to Central otherwise
 * Correctly refresh map after adding/editing map geometries (#11064)
+* Fix inconsistent state after user deletion failed (#11606)
 * Return embed private instead of 404 in visualization embeds where the visualization doesn't exist (#11056)
 * Fix error loading builder in visualizations without permissions (#10996)
 * Correctly update legend styles (with custom titles) (#10889, #10904)

app/controllers/admin/users_controller.rb

@@ -118,8 +118,8 @@ class Admin::UsersController < Admin::AdminController
       raise PASSWORD_DOES_NOT_MATCH_MESSAGE
     end
 
-    @user.delete_in_central
     @user.destroy
+    @user.delete_in_central
 
     redirect_to logout_url
   rescue CartoDB::CentralCommunicationFailure => e

app/controllers/carto/api/organization_users_controller.rb

@@ -121,13 +121,15 @@ module Carto
           render_jsonp "Can't delete @user. #{'Has shared entities' if @user.has_shared_entities?}", 410
         end
 
-        @user.delete_in_central
         @user.destroy
+        @user.delete_in_central
 
         render_jsonp 'User deleted', 200
       rescue CartoDB::CentralCommunicationFailure => e
         CartoDB::Logger.error(exception: e, message: 'Central error deleting user from EUMAPI', user: @user)
         render_jsonp "User couldn't be deleted", 500
+      rescue => e
+        render_jsonp "User couldn't be deleted: #{e.message}", 500
       end
 
       private

spec/requests/carto/api/organization_users_controller_spec.rb

@@ -555,9 +555,9 @@ describe Carto::Api::OrganizationUsersController do
         last_response.status.should eq 200
       end
 
-      it 'should not delete users that failed to delete from Central' do
-        ::User.any_instance.stubs(:destroy).never
-        mock_delete_request(500)
+      it 'should not delete users from Central that failed to delete in the box' do
+        ::User.any_instance.stubs(:delete_in_central).never
+        ::User.any_instance.stubs(:destroy).raises("BOOM")
         login(@organization.owner)
 
         delete api_v2_organization_users_delete_url(id_or_name: @organization.name,