Please linter

.github/workflows/rubocop.yml

@@ -12,5 +12,6 @@ jobs:
         uses: reviewdog/action-rubocop@v1
         with:
           github_token: ${{ secrets.github_token }}
-          reporter: github-pr-review # Optional. Default: github-pr-check
-          level: error # Optional. Default: error
+          reporter: github-pr-check
+          level: error
+          fail_on_error: true

.rubocop.yml

@@ -43,7 +43,7 @@ Metrics/AbcSize:
   Description: >-
                  A calculated magnitude based on number of assignments,
                  branches, and conditions.
-  Max: 30
+  Max: 35
 
 Metrics/BlockLength:
   Description: Avoid long blocks with many lines.
@@ -76,6 +76,14 @@ Metrics/PerceivedComplexity:
                   human reader.
   Max: 15
 
+####################
+#  Naming
+####################
+
+Naming/VariableNumber:
+  Description: Use the configured style when numbering variables.
+  Enabled: false
+
 ####################
 #  Style
 ####################

NEWS.md

@@ -64,6 +64,7 @@ Development
 * Fix visualization backup when permission is missing [#15874](https://github.com/CartoDB/cartodb/pull/15874)
 * Show outdated subscriptions. Optimize requests [#15879](https://github.com/CartoDB/cartodb/pull/15879)
 * Include `::Carto::ActiveRecordCompatibility` in all `Sequel` models [#15879](https://github.com/CartoDB/cartodb/pull/15879)
+* Migrate `Permission` model to `ActiveRecord` [#15878](https://github.com/CartoDB/cartodb/pull/15878)
 
 4.41.1 (2020-09-03)
 -------------------

app/controllers/carto/api/database_groups_controller.rb

@@ -158,13 +158,14 @@ module Carto
         @usernames = @username.present? ? [ @username ] : params[:users]
         @table_name = params[:table_name]
         case params['access']
-            when nil
-            when 'r'
-              @access = Carto::Permission::ACCESS_READONLY
-            when 'w'
-              @access = Carto::Permission::ACCESS_READWRITE
-            else raise "Unknown access #{params['access']}"
-            end
+        when 'r'
+          @access = Carto::Permission::ACCESS_READONLY
+        when 'w'
+          @access = Carto::Permission::ACCESS_READWRITE
+        when nil
+          nil
+        else raise "Unknown access #{params['access']}"
+        end
       end
 
       def get_group_from_loaded_parameters

app/models/carto/organization_permission.rb

@@ -9,9 +9,5 @@ module Carto
       table.add_organization_read_write_permission
     end
 
-    def is_permitted(table, access)
-      table.permission.permission_for_org == access
-    end
-
   end
 end

app/models/carto/permission.rb

@@ -28,24 +28,36 @@ class Carto::Permission < ActiveRecord::Base
     temp_old_acl = {}
     # Convert the old and new acls to a better format for searching
     old_acl.each do |i|
-      if !temp_old_acl.has_key?(i[:type])
-        temp_old_acl[i[:type]] = {}
-      end
+      temp_old_acl[i[:type]] = {} unless temp_old_acl.key?(i[:type])
       temp_old_acl[i[:type]][i[:id]] = i
     end
     temp_new_acl = {}
     new_acl.each do |i|
-      if !temp_new_acl.has_key?(i[:type])
-        temp_new_acl[i[:type]] = {}
-      end
+      temp_new_acl[i[:type]] = {} unless temp_new_acl.key?(i[:type])
       temp_new_acl[i[:type]][i[:id]] = i
     end
 
-    # Iterate through the new acl and compare elements with the old one
-    permissions_change = {}
+    changed_permissions = build_changed_permissions(temp_new_acl, temp_old_acl)
+
+    # Iterate through the old acl. All the permissions in this structure are
+    # supposed so be revokes
+    temp_old_acl.each do |pt, pv|
+      changed_permissions[pt] = {} if changed_permissions[pt].nil?
+
+      pv.each do |oi, _iacl|
+        changed_permissions[pt][oi] = [{ 'action' => 'revoke', 'type' => temp_old_acl[pt][oi][:access] }]
+      end
+    end
+
+    changed_permissions
+  end
+
+  def self.build_changed_permissions(temp_new_acl, temp_old_acl)
+    changed_permissions = {}
+
     temp_new_acl.each do |pt, pv|
-      permissions_change[pt] = {}
-      pv.each do |oi, iacl|
+      changed_permissions[pt] = {}
+      pv.each do |oi, _iacl|
         # See if a specific permission exists in the old acl
         # If the new acl is greater than the old we suppose that write
         # permissions were granted. Otherwise they were revoked
@@ -55,30 +67,19 @@ class Carto::Permission < ActiveRecord::Base
         if !temp_old_acl[pt].nil? && !temp_old_acl[pt][oi].nil?
           case temp_new_acl[pt][oi][:access] <=> temp_old_acl[pt][oi][:access]
           when 1
-            permissions_change[pt][oi] = [{'action' => 'grant', 'type' => 'w'}]
+            changed_permissions[pt][oi] = [{ 'action' => 'grant', 'type' => 'w' }]
           when -1
-            permissions_change[pt][oi] = [{'action' => 'revoke', 'type' => 'w'}]
+            changed_permissions[pt][oi] = [{ 'action' => 'revoke', 'type' => 'w' }]
           end
           temp_old_acl[pt].delete(oi)
         else
-          permissions_change[pt][oi] = [{'action' => 'grant', 'type' => temp_new_acl[pt][oi][:access]}]
+          changed_permissions[pt][oi] = [{ 'action' => 'grant', 'type' => temp_new_acl[pt][oi][:access] }]
         end
         temp_new_acl[pt].delete(oi)
       end
     end
 
-    # Iterate through the old acl. All the permissions in this structure are
-    # supposed so be revokes
-    temp_old_acl.each do |pt, pv|
-      if permissions_change[pt].nil?
-        permissions_change[pt] = {}
-      end
-      pv.each do |oi, iacl|
-        permissions_change[pt][oi] = [{'action' => 'revoke', 'type' => temp_old_acl[pt][oi][:access]}]
-      end
-    end
-
-    return permissions_change
+    changed_permissions
   end
 
   def acl
@@ -147,26 +148,20 @@ class Carto::Permission < ActiveRecord::Base
   # @throws Carto::Permission::Error
   def acl=(value)
     incoming_acl = value.nil? ? DEFAULT_ACL_VALUE : value
-    raise Carto::Permission::Error.new('ACL is not an array') unless incoming_acl.is_a? Array
+    raise Carto::Permission::Error, 'ACL is not an array' unless incoming_acl.is_a? Array
+
     incoming_acl.map do |item|
       unless item.is_a?(Hash) && acl_has_required_fields?(item) && acl_has_valid_access?(item)
-        raise Carto::Permission::Error.new('Wrong ACL entry format')
+        raise Carto::Permission::Error, 'Wrong ACL entry format'
       end
     end
 
     acl_items = incoming_acl.map do |item|
-      {
-        type:   item[:type],
-        id:     item[:entity][:id],
-        access: item[:access]
-      }
+      { type: item[:type], id: item[:entity][:id], access: item[:access] }
     end
 
     cleaned_acl = acl_items.select { |i| i[:id] } # Cleaning, see #5668
-
-    if @old_acl.nil?
-      @old_acl = acl
-    end
+    @old_acl ||= acl
 
     self.access_control_list = ::JSON.dump(cleaned_acl)
   end
@@ -302,11 +297,9 @@ class Carto::Permission < ActiveRecord::Base
 
   def permission_for_org
     permission = nil
-    acl.map { |entry|
-      if entry[:type] == TYPE_ORGANIZATION
-          permission = entry[:access]
-      end
-    }
+    acl.map do |entry|
+      permission = entry[:access] if entry[:type] == TYPE_ORGANIZATION
+    end
     ACCESS_NONE if permission.nil?
   end
 
@@ -568,13 +561,13 @@ class Carto::Permission < ActiveRecord::Base
       # assert database permissions for non canonical tables are assigned
       # its canonical vis
       if not entity.table
-        raise Carto::Permission::Error.new('Trying to change permissions to a table without ownership')
+        raise Carto::Permission::Error, 'Trying to change permissions to a table without ownership'
       end
       table = entity.table
 
       # check ownership
       if not owner_id == entity.permission.owner_id
-        raise Carto::Permission::Error.new('Trying to change permissions to a table without ownership')
+        raise Carto::Permission::Error, 'Trying to change permissions to a table without ownership'
       end
       # give permission
       if access == ACCESS_READONLY
@@ -583,7 +576,7 @@ class Carto::Permission < ActiveRecord::Base
         permission_strategy.add_read_write_permission(table)
       end
     else
-      raise Carto::Permission::Error.new('Unsupported entity type trying to grant permission')
+      raise Carto::Permission::Error, 'Unsupported entity type trying to grant permission'
     end
   end
 

app/models/carto/user_permission.rb

@@ -5,10 +5,6 @@ module Carto
       @user = user
     end
 
-    def is_permitted(table, access)
-      table.permission.permitted?(@user, access)
-    end
-
     def add_read_permission(table)
       table.add_read_permission(@user)
     end

spec/factories/carto_visualizations.rb

@@ -7,8 +7,11 @@ module Carto
 
       def full_visualization_table(carto_user, map)
         carto_user = Carto::User.find(carto_user.id) unless carto_user.is_a? Carto::User
-        map_id = map.nil? ? nil : map.id
-        Carto::UserTable.find(create_table(name: unique_name('fvt_table'), user_id: carto_user.id, map_id: map_id).id)
+        Carto::UserTable.find(create_table(
+          name: unique_name('fvt_table'),
+          user_id: carto_user.id,
+          map_id: map&.id
+        ).id)
       end
 
       def create_full_builder_vis(carto_user, visualization_attributes: {})
@@ -17,41 +20,36 @@ module Carto
 
       # "Full visualization": with map, table... Including actual user table.
       # Table is bound to visualization, and to data_layer if it's not passed.
-      def create_full_visualization(
-        carto_user,
-        canonical_map: FactoryGirl.create(:carto_map_with_layers, user_id: carto_user.id),
-        map: FactoryGirl.create(:carto_map_with_layers, user_id: carto_user.id),
-        table: full_visualization_table(carto_user, canonical_map),
-        data_layer: nil,
-        visualization_attributes: {}
-      )
-
-        carto_user = Carto::User.find(carto_user.id) unless carto_user.is_a? Carto::User
-
+      def create_full_visualization(carto_user, params = {})
+        carto_user = carto_user.carto_user
+        canonical_map = params[:canonical_map] || create(:carto_map_with_layers, user_id: carto_user.id)
+        map = params[:map] || create(:carto_map_with_layers, user_id: carto_user.id)
+        table = params[:table] || full_visualization_table(carto_user, canonical_map)
         table_visualization = table.visualization || create_table_visualization(carto_user, table)
-        visualization = FactoryGirl.create(:carto_visualization, { user: carto_user, map: map }
-                                   .merge(visualization_attributes))
+        visualization_attributes = { user: carto_user, map: map }
+        visualization_attributes.merge(params[:visualization_attributes]) if params[:visualization_attributes]
+        visualization = create(:carto_visualization, visualization_attributes)
 
-        unless data_layer.present?
-          data_layer = visualization.map.data_layers.first
-          data_layer.options[:table_name] = table.name
-          data_layer.options[:query] = "select * from #{table.name}"
-          data_layer.options[:sql_wrap] = "select * from (<%= sql %>) __wrap"
-          data_layer.save
-        end
+        build_data_layer(visualization, table) unless params[:data_layer]
 
         visualization.update_column(:active_layer_id, visualization.layers.first.id)
 
-        FactoryGirl.create(:carto_zoom_overlay, visualization: visualization)
-        FactoryGirl.create(:carto_search_overlay, visualization: visualization)
+        create(:carto_zoom_overlay, visualization: visualization)
+        create(:carto_search_overlay, visualization: visualization)
 
         # Need to mock the nonexistant table because factories use Carto::* models
         CartoDB::Visualization::Member.any_instance.stubs(:propagate_name_to).returns(true)
         CartoDB::Visualization::Member.any_instance.stubs(:propagate_privacy_to).returns(true)
 
-        visualization.reload
+        [map, table, table_visualization, visualization.reload]
+      end
 
-        return map, table, table_visualization, visualization
+      def build_data_layer(visualization, table)
+        data_layer = visualization.map.data_layers.first
+        data_layer.options[:table_name] = table.name
+        data_layer.options[:query] = "select * from #{table.name}"
+        data_layer.options[:sql_wrap] = 'select * from (<%= sql %>) __wrap'
+        data_layer.save
       end
 
       def create_table_visualization(carto_user, table)

spec/factories/organizations_contexts.rb

@@ -1,13 +1,14 @@
 require_relative '../support/factories/users'
 require 'helpers/unique_names_helper'
 
-include UniqueNamesHelper
-
 class TestUserFactory
+
   include CartoDB::Factories
+
 end
 
 module TableSharing
+
   def share_table_with_user(table, user, access: Carto::Permission::ACCESS_READONLY)
     vis = CartoDB::Visualization::Member.new(id: table.table_visualization.id).fetch
     per = vis.permission
@@ -23,11 +24,14 @@ module TableSharing
     per.save
     per.reload
   end
+
 end
 
 shared_context 'organization with users helper' do
   include CacheHelper
   include CartoDB::Factories
+  include UniqueNamesHelper
+
   include_context 'database configuration'
 
   before(:each) do
@@ -85,17 +89,23 @@ shared_context 'organization with users helper' do
 
   def share_table(table, owner, user)
     bypass_named_maps
-    headers = {'CONTENT_TYPE'  => 'application/json'}
+    headers = { 'CONTENT_TYPE' => 'application/json' }
     perm_id = table.table_visualization.permission.id
 
-    put api_v1_permissions_update_url(user_domain: owner.username, api_key: owner.api_key, id: perm_id),
-        { acl: [{
-                 type: Carto::Permission::TYPE_USER,
-                 entity: {
-                   id:   user.id,
-                 },
-                 access: Carto::Permission::ACCESS_READONLY
-               }]}.to_json, headers
+    request_payload = {
+      acl: [
+        {
+          type: Carto::Permission::TYPE_USER,
+          entity: { id: user.id },
+          access: Carto::Permission::ACCESS_READONLY
+        }
+      ]
+    }.to_json
+    put(
+      api_v1_permissions_update_url(user_domain: owner.username, api_key: owner.api_key, id: perm_id),
+      request_payload,
+      headers
+    )
     response.status.should == 200
   end
 

spec/models/visualization/member_spec.rb

@@ -10,7 +10,6 @@ require_dependency 'cartodb/redis_vizjson_cache'
 include CartoDB
 
 describe Visualization::Member do
-
   include Carto::Factories::Visualizations
   let(:user) { create(:carto_user) }
 

spec/requests/carto/api/database_groups_controller_spec.rb

@@ -102,14 +102,11 @@ describe Carto::Api::DatabaseGroupsController do
       permission.should_not be_nil
 
       expected_acl = [
-          {
-              type: Carto::Permission::TYPE_GROUP,
-              entity: {
-                  id:         group.id,
-                  name:       group.name
-              },
-              access: Carto::Permission::ACCESS_READONLY
-          }
+        {
+          type: Carto::Permission::TYPE_GROUP,
+          entity: { id: group.id, name: group.name },
+          access: Carto::Permission::ACCESS_READONLY
+        }
       ]
       permission.to_poro[:acl].should == expected_acl
 
@@ -181,17 +178,23 @@ describe Carto::Api::DatabaseGroupsController do
     it '#update_permission granting read on a table to organization, group and user do not duplicate count' do
       bypass_named_maps
       @table_user_2 = create_table_with_options(@org_user_2)
-      put api_v1_permissions_update_url(user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: @table_user_2['table_visualization'][:permission][:id]),
-          { acl: [ {
-              type: Carto::Permission::TYPE_USER,
-              entity: { id:   @org_user_1.id },
-              access: Carto::Permission::ACCESS_READONLY
-            }, {
-              type: Carto::Permission::TYPE_ORGANIZATION,
-              entity: { id:   @organization.id },
-              access: Carto::Permission::ACCESS_READONLY
-            } ]
-          }.to_json, http_json_headers
+      request_payload = {
+        acl: [
+          {
+            type: Carto::Permission::TYPE_USER,
+            entity: { id: @org_user_1.id },
+            access: Carto::Permission::ACCESS_READONLY
+          },
+          {
+            type: Carto::Permission::TYPE_ORGANIZATION,
+            entity: { id: @organization.id },
+            access: Carto::Permission::ACCESS_READONLY
+          }
+        ]
+      }.to_json
+      permission_id = @table_user_2['table_visualization'][:permission][:id]
+      request_url_params = { user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: permission_id }
+      put api_v1_permissions_update_url(request_url_params), request_payload, http_json_headers
       response.status.should == 200
 
       group = Carto::Group.where(organization_id: @carto_organization.id).first

spec/requests/carto/api/permissions_controller_spec.rb

@@ -44,31 +44,29 @@ describe Carto::Api::PermissionsController do
     it 'modifies an existing permission' do
       entity_type = Carto::Permission::ENTITY_TYPE_VISUALIZATION
 
-      acl_initial = [ ]
+      acl_initial = []
       client_acl_modified = [
         {
           type: Carto::Permission::TYPE_USER,
-          entity: {
-            id:   @user2.id,
-          },
+          entity: { id: @user2.id },
           access: Carto::Permission::ACCESS_READONLY
         }
       ]
       client_acl_modified_expected = [
-          {
-              type: Carto::Permission::TYPE_USER,
-              entity: {
-                  id:         @user2.id,
-                  username:   @user2.username,
-                  avatar_url: @user2.avatar_url,
-                  viewer:     false,
-                  base_url:   @user2.public_url,
-                  groups:     []
-              },
-              access: Carto::Permission::ACCESS_READONLY
-          }
+        {
+          type: Carto::Permission::TYPE_USER,
+          entity: {
+            id: @user2.id,
+            username: @user2.username,
+            avatar_url: @user2.avatar_url,
+            viewer: false,
+            base_url: @user2.public_url,
+            groups: []
+          },
+          access: Carto::Permission::ACCESS_READONLY
+        }
       ]
-      client_acl_final = [ ]
+      client_acl_final = []
 
       permission = @visualization.permission
       permission.owner_id.should eq @user.id
@@ -162,14 +160,11 @@ describe 'group permission support' do
       }
     ]
     client_acl_modified_expected = [
-        {
-            type: Carto::Permission::TYPE_GROUP,
-            entity: {
-                id:         @group.id,
-                name:       @group.name
-            },
-            access: Carto::Permission::ACCESS_READONLY
-        }
+      {
+        type: Carto::Permission::TYPE_GROUP,
+        entity: { id: @group.id, name: @group.name },
+        access: Carto::Permission::ACCESS_READONLY
+      }
     ]
 
     permission = visualization.permission

spec/requests/carto/api/visualizations_controller_spec_part_2.rb

@@ -211,14 +211,20 @@ describe Carto::Api::VisualizationsController do
       body['visualizations'][0]['id'].should eq u1_vis_1_id
 
       # Share u1 table with u2
-      put api_v1_permissions_update_url(user_domain:user_1.username, api_key: user_1.api_key, id: u1_t_1_perm_id),
-          {acl: [{
-                   type: Carto::Permission::TYPE_USER,
-                   entity: {
-                     id:   user_2.id,
-                   },
-                   access: Carto::Permission::ACCESS_READONLY
-                 }]}.to_json, @headers
+      request_payload = {
+        acl: [
+          {
+            type: Carto::Permission::TYPE_USER,
+            entity: { id: user_2.id },
+            access: Carto::Permission::ACCESS_READONLY
+          }
+        ]
+      }.to_json
+      put(
+        api_v1_permissions_update_url(user_domain: user_1.username, api_key: user_1.api_key, id: u1_t_1_perm_id),
+        request_payload,
+        @headers
+      )
       last_response.status.should == 200
 
       # Dunno why (rack test error?) but this call seems to cache previous params, so just call it to "flush" them
@@ -359,14 +365,17 @@ describe Carto::Api::VisualizationsController do
         vis['id'].should eq u1_vis_1_id
 
         # Share u2 vis1 with organization
-        put api_v1_permissions_update_url(user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_vis_1_perm_id),
-            {acl: [{
-                       type: Carto::Permission::TYPE_ORGANIZATION,
-                       entity: {
-                           id:   @organization.id,
-                       },
-                       access: Carto::Permission::ACCESS_READONLY
-                   }]}.to_json, @headers
+        request_payload = {
+          acl: [
+            {
+              type: Carto::Permission::TYPE_ORGANIZATION,
+              entity: { id: @organization.id },
+              access: Carto::Permission::ACCESS_READONLY
+            }
+          ]
+        }.to_json
+        request_url_params = { user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_vis_1_perm_id }
+        put api_v1_permissions_update_url(request_url_params), request_payload, @headers
         last_response.status.should == 200
 
         get api_v1_visualizations_index_url(user_domain: @org_user_1.username, api_key: @org_user_1.api_key,
@@ -377,14 +386,17 @@ describe Carto::Api::VisualizationsController do
         body['total_locked'].should eq 0
 
         # Share u2 vis2 with u1
-        put api_v1_permissions_update_url(user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_vis_2_perm_id),
-            {acl: [{
-                       type: Carto::Permission::TYPE_USER,
-                       entity: {
-                           id:   @org_user_1.id,
-                       },
-                       access: Carto::Permission::ACCESS_READONLY
-                   }]}.to_json, @headers
+        request_payload = {
+          acl: [
+            {
+              type: Carto::Permission::TYPE_USER,
+              entity: { id: @org_user_1.id },
+              access: Carto::Permission::ACCESS_READONLY
+            }
+          ]
+        }.to_json
+        request_url_params = { user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_vis_2_perm_id }
+        put api_v1_permissions_update_url(request_url_params), request_payload, @headers
         last_response.status.should == 200
 
         get api_v1_visualizations_index_url(user_domain: @org_user_1.username, api_key: @org_user_1.api_key,
@@ -413,16 +425,18 @@ describe Carto::Api::VisualizationsController do
         body['total_shared'].should eq 2
         body['total_locked'].should eq 0
 
-
         # Share u2 table1 with org
-        put api_v1_permissions_update_url(user_domain:@org_user_2.username, api_key: @org_user_2.api_key, id: u2_t_1_perm_id),
-            {acl: [{
-                       type: Carto::Permission::TYPE_ORGANIZATION,
-                       entity: {
-                           id:   @organization.id,
-                       },
-                       access: Carto::Permission::ACCESS_READONLY
-                   }]}.to_json, @headers
+        payload = {
+          acl: [
+            {
+              type: Carto::Permission::TYPE_ORGANIZATION,
+              entity: { id: @organization.id },
+              access: Carto::Permission::ACCESS_READONLY
+            }
+          ]
+        }.to_json
+        url_params = { user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_t_1_perm_id }
+        put api_v1_permissions_update_url(url_params), payload, @headers
 
         get api_v1_visualizations_index_url(user_domain: @org_user_1.username, api_key: @org_user_1.api_key,
                                             type: CartoDB::Visualization::Member::TYPE_CANONICAL, order: 'updated_at'), @headers
@@ -432,14 +446,17 @@ describe Carto::Api::VisualizationsController do
         body['total_locked'].should eq 0
 
         # Share u2 table2 with org
-        put api_v1_permissions_update_url(user_domain:@org_user_2.username, api_key: @org_user_2.api_key, id: u2_t_2_perm_id),
-            {acl: [{
-                       type: Carto::Permission::TYPE_USER,
-                       entity: {
-                           id:   @org_user_1.id,
-                       },
-                       access: Carto::Permission::ACCESS_READONLY
-                   }]}.to_json, @headers
+        request_payload = {
+          acl: [
+            {
+              type: Carto::Permission::TYPE_USER,
+              entity: { id: @org_user_1.id },
+              access: Carto::Permission::ACCESS_READONLY
+            }
+          ]
+        }.to_json
+        request_url_params = { user_domain: @org_user_2.username, api_key: @org_user_2.api_key, id: u2_t_2_perm_id }
+        put api_v1_permissions_update_url(request_url_params), request_payload, @headers
 
         get api_v1_visualizations_index_url(user_domain: @org_user_1.username, api_key: @org_user_1.api_key,
                                             type: CartoDB::Visualization::Member::TYPE_CANONICAL, order: 'updated_at'), @headers

spec/requests/carto/api/vizjson_shared_examples.rb

@@ -86,12 +86,17 @@ shared_examples_for 'vizjson generator' do
       last_response.status.should == 200
 
       # Share vis with user_2 in readonly (vis can never be shared in readwrite)
-      put api_v1_permissions_update_url(user_domain: user_1.username, api_key: user_1.api_key, id: u1_vis_1_perm_id),
-          { acl: [{
-                   type: Carto::Permission::TYPE_USER,
-                   entity: { id:   user_2.id },
-                   access: Carto::Permission::ACCESS_READONLY
-                  }] }.to_json, http_json_headers
+      request_payload = {
+        acl: [
+          {
+            type: Carto::Permission::TYPE_USER,
+            entity: { id: user_2.id },
+            access: Carto::Permission::ACCESS_READONLY
+          }
+        ]
+      }.to_json
+      request_url_params = { user_domain: user_1.username, api_key: user_1.api_key, id: u1_vis_1_perm_id }
+      put api_v1_permissions_update_url(request_url_params), request_payload, http_json_headers
       last_response.status.should == 200
 
       # privacy private checks