Member -> user renaming refactor

app/controllers/carto/api/database_groups_controller.rb

@@ -12,13 +12,13 @@ module Carto
 
       respond_to :json
 
-      ssl_required :create, :update, :destroy, :add_member, :remove_member, :update_permission, :destroy_permission unless Rails.env.development? || Rails.env.test?
+      ssl_required :create, :update, :destroy, :add_users, :remove_users, :update_permission, :destroy_permission unless Rails.env.development? || Rails.env.test?
 
       before_filter :authenticate_extension
       before_filter :load_parameters
-      before_filter :load_mandatory_group, :only => [:destroy, :add_member, :remove_member, :update_permission, :destroy_permission]
+      before_filter :load_mandatory_group, :only => [:destroy, :add_users, :remove_users, :update_permission, :destroy_permission]
       before_filter :load_user_from_username, :only => [:load_table, :update_permission, :destroy_permission]
-      before_filter :load_users_from_username, :only => [:add_member, :remove_member]
+      before_filter :load_users_from_username, :only => [:add_users, :remove_users]
       before_filter :load_table, :only => [:update_permission, :destroy_permission]
 
       def create
@@ -68,11 +68,11 @@ module Carto
         render json: { errors: e.message }, status: 500
       end
 
-      def add_member
+      def add_users
         added_usernames = []
         @usernames.map { |username|
           begin
-            added_usernames << @group.add_member(username).user.username
+            added_usernames << @group.add_user(username).user.username
           rescue CartoDB::ModelAlreadyExistsError => e
             # This will provoke 409 response later
           end
@@ -87,10 +87,10 @@ module Carto
         render json: { errors: e.message }, status: 500
       end
 
-      def remove_member
+      def remove_users
         removed_usernames = []
         @usernames.map { |username|
-          removed_user = @group.remove_member(username)
+          removed_user = @group.remove_user(username)
           removed_usernames << removed_user.username if !removed_user.nil?
         }
         if removed_usernames.length == @usernames.length

app/controllers/carto/api/group_presenter.rb

@@ -7,7 +7,7 @@ module Carto
       # Available fetching options:
       # - fetch_shared_tables_count
       # - fetch_shared_maps_count
-      # - fetch_members
+      # - fetch_users
       def initialize(group, fetching_options = {})
         @group = group
         @fetching_options = fetching_options
@@ -27,8 +27,8 @@ module Carto
         if @fetching_options[:fetch_shared_maps_count] == true
           poro.merge!({ shared_maps_count: shared_maps_count })
         end
-        if @fetching_options[:fetch_members] == true
-          poro.merge!({ members: members })
+        if @fetching_options[:fetch_users] == true
+          poro.merge!({ users: users })
         end
 
         poro
@@ -48,7 +48,7 @@ module Carto
         Carto::SharedEntity.where(recipient_id: @group.id).joins(:visualization)
       end
 
-      def members
+      def users
         @group.users.map { |u| Carto::Api::UserPresenter.new(u, { fetch_groups: false } ).to_poro }
       end
 

app/controllers/carto/api/groups_controller.rb

@@ -9,16 +9,16 @@ module Carto
     class GroupsController < ::Api::ApplicationController
       include PagedSearcher
 
-      ssl_required :index, :show, :create, :update, :destroy, :add_members, :remove_members unless Rails.env.development? || Rails.env.test?
+      ssl_required :index, :show, :create, :update, :destroy, :add_users, :remove_users unless Rails.env.development? || Rails.env.test?
 
       before_filter :load_fetching_options, :only => [:show, :index]
       before_filter :load_organization
       before_filter :load_user
       before_filter :validate_organization_or_user_loaded
-      before_filter :load_group, :only => [:show, :update, :destroy, :add_members, :remove_members]
-      before_filter :org_owner_only, :only => [:create, :update, :destroy, :add_members, :remove_members]
+      before_filter :load_group, :only => [:show, :update, :destroy, :add_users, :remove_users]
+      before_filter :org_owner_only, :only => [:create, :update, :destroy, :add_users, :remove_users]
       before_filter :org_users_only, :only => [:show, :index]
-      before_filter :load_organization_users, :only => [:add_members, :remove_members]
+      before_filter :load_organization_users, :only => [:add_users, :remove_users]
 
       def index
         page, per_page, order = page_per_page_order_params
@@ -66,9 +66,9 @@ module Carto
         render json: { errors: e.message }, status: 500
       end
 
-      def add_members
+      def add_users
         @organization_users.map { |user|
-          @group.add_member_with_extension(user)
+          @group.add_user_with_extension(user)
         }
         render json: {}, status: 200
       rescue => e
@@ -76,9 +76,9 @@ module Carto
         render json: { errors: e.message }, status: 500
       end
 
-      def remove_members
+      def remove_users
         @organization_users.map { |user|
-          @group.remove_member_with_extension(user)
+          @group.remove_user_with_extension(user)
         }
         render json: {}, status: 200
       rescue => e
@@ -92,7 +92,7 @@ module Carto
         @fetching_options = {
           fetch_shared_tables_count: params[:fetch_shared_tables_count] == 'true',
           fetch_shared_maps_count: params[:fetch_shared_maps_count] == 'true',
-          fetch_members: params[:fetch_members] == 'true'
+          fetch_users: params[:fetch_users] == 'true'
         }
       end
 

app/models/carto/group.rb

@@ -13,8 +13,8 @@ module Carto
   # - create_group_with_extension
   # - rename_group_with_extension
   # - destroy_group_with_extension
-  # - add_member_with_extension
-  # - remove_member_with_extension
+  # - add_user_with_extension
+  # - remove_user_with_extension
   class Group < ActiveRecord::Base
     include PagedModel
 
@@ -70,16 +70,16 @@ module Carto
       end
     end
 
-    def add_member_with_extension(user)
+    def add_user_with_extension(user)
       organization.owner.in_database do |conn|
-        Carto::Group.add_member_group_extension_query(conn, name, user.username)
+        Carto::Group.add_user_group_extension_query(conn, name, user.username)
       end
       self.reload
     end
 
-    def remove_member_with_extension(user)
+    def remove_user_with_extension(user)
       organization.owner.in_database do |conn|
-        Carto::Group.remove_member_group_extension_query(conn, name, user.username)
+        Carto::Group.remove_user_group_extension_query(conn, name, user.username)
       end
       self.reload
     end
@@ -89,7 +89,7 @@ module Carto
       self.database_role = new_database_role
     end
 
-    def add_member(username)
+    def add_user(username)
       user = Carto::User.find_by_username(username)
 
       raise "User #{username} not found" unless user
@@ -101,7 +101,7 @@ module Carto
       user_group
     end
 
-    def remove_member(username)
+    def remove_user(username)
       user = Carto::User.find_by_username(username)
 
       raise "User #{username} not found" unless user
@@ -139,11 +139,11 @@ module Carto
       conn.execute(%Q{ select cartodb.CDB_Group_DropGroup('#{name}') })
     end
 
-    def self.add_member_group_extension_query(conn, name, username)
+    def self.add_user_group_extension_query(conn, name, username)
       conn.execute(%Q{ select cartodb.CDB_Group_AddMember('#{name}', '#{username}') })
     end
 
-    def self.remove_member_group_extension_query(conn, name, username)
+    def self.remove_user_group_extension_query(conn, name, username)
       conn.execute(%Q{ select cartodb.CDB_Group_RemoveMember('#{name}', '#{username}') })
     end
 

config/routes.rb

@@ -376,16 +376,16 @@ CartoDB::Application.routes.draw do
 
     get '(/user/:user_domain)(/u/:user_domain)/api/v1/users/:user_id/groups' => 'groups#index', as: :api_v1_user_groups, constraints: { user_id: /[^\/]+/ }
 
-    post '(/user/:user_domain)(/u/:user_domain)/api/v1/organization/:organization_id/groups/:group_id/users' => 'groups#add_members', as: :api_v1_organization_groups_add_members, constraints: { organization_id: /[^\/]+/, group_id: /[^\/]+/ }
-    delete '(/user/:user_domain)(/u/:user_domain)/api/v1/organization/:organization_id/groups/:group_id/users(/:user_id)' => 'groups#remove_members', as: :api_v1_organization_groups_remove_members, constraints: { organization_id: /[^\/]+/, group_id: /[^\/]+/ , user_id: /[^\/]+/ }
+    post '(/user/:user_domain)(/u/:user_domain)/api/v1/organization/:organization_id/groups/:group_id/users' => 'groups#add_users', as: :api_v1_organization_groups_add_users, constraints: { organization_id: /[^\/]+/, group_id: /[^\/]+/ }
+    delete '(/user/:user_domain)(/u/:user_domain)/api/v1/organization/:organization_id/groups/:group_id/users(/:user_id)' => 'groups#remove_users', as: :api_v1_organization_groups_remove_users, constraints: { organization_id: /[^\/]+/, group_id: /[^\/]+/ , user_id: /[^\/]+/ }
 
     # Databases (organization) groups
     # Note: url doesn't contain org_id because this needs to be triggered from the SQL API
     post '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups'                           => 'database_groups#create',  as: :api_v1_databases_group_create
     put '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:old_name'          => 'database_groups#update',  as: :api_v1_databases_group_update
     delete '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name'          => 'database_groups#destroy',  as: :api_v1_databases_group_destroy
-    post '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/users'          => 'database_groups#add_member',  as: :api_v1_databases_group_add_member
-    delete '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/users(/:username)'          => 'database_groups#remove_member',  as: :api_v1_databases_group_remove_member
+    post '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/users'          => 'database_groups#add_users',  as: :api_v1_databases_group_add_users
+    delete '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/users(/:username)'          => 'database_groups#remove_users',  as: :api_v1_databases_group_remove_users
     put '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/permission/:username/tables/:table_name' => 'database_groups#update_permission', as: :api_v1_databases_group_update_permission
     delete '(/user/:user_domain)(/u/:user_domain)/api/v1/databases/:database_name/groups/:name/permission/:username/tables/:table_name' => 'database_groups#destroy_permission', as: :api_v1_databases_group_destroy_permission
 

spec/requests/carto/api/database_groups_controller_spec.rb

@@ -59,19 +59,19 @@ describe Carto::Api::DatabaseGroupsController do
       response.status.should == 500
     end
 
-    it '#add_member from username' do
+    it '#add_users from username' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       user_information = { username: @org_user_1.username }
-      post api_v1_databases_group_add_member_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
+      post api_v1_databases_group_add_users_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
       response.status.should == 200
       group.reload
       group.users.collect(&:username).should include(@org_user_1.username)
     end
 
-    it '#add_member returns 409 if username is already added' do
+    it '#add_users returns 409 if username is already added' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       user_information = { username: @org_user_1.username }
-      post api_v1_databases_group_add_member_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
+      post api_v1_databases_group_add_users_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
       response.status.should == 409
     end
 
@@ -167,38 +167,36 @@ describe Carto::Api::DatabaseGroupsController do
       response.status.should == 404
     end
 
-    # TODO: support for tables not yet registered?
-
-    it '#remove_member from username' do
+    it '#remove_users from username' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       username = group.users.first.username
-      delete api_v1_databases_group_remove_member_url(database_name: group.database_name, name: group.name, username: username), {}, org_metadata_api_headers
+      delete api_v1_databases_group_remove_users_url(database_name: group.database_name, name: group.name, username: username), {}, org_metadata_api_headers
       response.status.should == 200
       group.reload
       group.users.collect(&:username).should_not include(username)
     end
 
-    it '#remove_member from username throws 404 if member is not found' do
+    it '#remove_users from username throws 404 if user is not found' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       username = @org_user_1.username
-      delete api_v1_databases_group_remove_member_url(database_name: group.database_name, name: group.name, username: username), {}, org_metadata_api_headers
+      delete api_v1_databases_group_remove_users_url(database_name: group.database_name, name: group.name, username: username), {}, org_metadata_api_headers
       response.status.should == 404
     end
 
-    it '#add_member from username accepts batches' do
+    it '#add_users from username accepts batches' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       user_information = { users: [ @org_user_1.username, @org_user_2.username ] }
-      post api_v1_databases_group_add_member_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
+      post api_v1_databases_group_add_users_url(database_name: group.database_name, name: group.name), user_information.to_json, org_metadata_api_headers
       response.status.should == 200
       group.reload
       group.users.collect(&:username).should include(@org_user_1.username)
       group.users.collect(&:username).should include(@org_user_2.username)
     end
 
-    it '#remove_member from username accepts batches' do
+    it '#remove_users from username accepts batches' do
       group = Carto::Group.where(organization_id: @carto_organization.id).first
       usernames = group.users.collect(&:username)
-      delete_json api_v1_databases_group_remove_member_url(database_name: group.database_name, name: group.name), { users: usernames }, org_metadata_api_headers
+      delete_json api_v1_databases_group_remove_users_url(database_name: group.database_name, name: group.name), { users: usernames }, org_metadata_api_headers
       response.status.should == 200
       group.reload
       usernames.map { |username|

spec/requests/carto/api/groups_controller_spec.rb

@@ -79,11 +79,11 @@ describe Carto::Api::GroupsController do
       describe "users groups" do
 
         before(:each) do
-          @group_1.add_member(@org_user_1.username)
+          @group_1.add_user(@org_user_1.username)
         end
 
         after(:each) do
-          @group_1.remove_member(@org_user_1.username)
+          @group_1.remove_user(@org_user_1.username)
         end
 
         it 'returns user groups if user_id is requested' do
@@ -97,16 +97,16 @@ describe Carto::Api::GroupsController do
           end
         end
 
-        it 'can fetch number of shared tables, maps and members' do
+        it 'can fetch number of shared tables, maps and users' do
 
-          get_json api_v1_user_groups_url(user_domain: @org_user_1.username, user_id: @org_user_1.id, api_key: @org_user_1.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_members: true), {}, @headers do |response|
+          get_json api_v1_user_groups_url(user_domain: @org_user_1.username, user_id: @org_user_1.id, api_key: @org_user_1.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_users: true), {}, @headers do |response|
             response.status.should == 200
             expected_response = {
               groups: [
                 @group_1_json.merge({
                   'shared_tables_count' => 0,
                   'shared_maps_count' => 0,
-                  'members' => [ @org_user_1_json ]
+                  'users' => [ @org_user_1_json ]
                 })
               ],
               total_entries: 1
@@ -115,21 +115,21 @@ describe Carto::Api::GroupsController do
           end
         end
 
-        it 'can fetch number of shared tables, maps and members when a table is shared' do
+        it 'can fetch number of shared tables, maps and users when a table is shared' do
           bypass_named_maps
           table_user_2 = create_table_with_options(@org_user_2)
           permission = CartoDB::Permission[Carto::Visualization.find(table_user_2['table_visualization']['id']).permission.id]
           permission.set_group_permission(@group_1, Carto::Permission::ACCESS_READONLY)
           permission.save
 
-          get_json api_v1_user_groups_url(user_domain: @org_user_1.username, user_id: @org_user_1.id, api_key: @org_user_1.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_members: true), {}, @headers do |response|
+          get_json api_v1_user_groups_url(user_domain: @org_user_1.username, user_id: @org_user_1.id, api_key: @org_user_1.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_users: true), {}, @headers do |response|
             response.status.should == 200
             expected_response = {
               groups: [
                 @group_1_json.merge({
                   'shared_tables_count' => 1,
                   'shared_maps_count' => 0,
-                  'members' => [ @org_user_1_json ]
+                  'users' => [ @org_user_1_json ]
                 })
               ],
               total_entries: 1
@@ -149,12 +149,12 @@ describe Carto::Api::GroupsController do
       end
     end
 
-    it '#show support fetch_shared_maps_count, fetch_shared_tables_count and fetch_members' do
-      get_json api_v1_organization_groups_show_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: @group_1.id, api_key: @org_user_owner.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_members: true), { }, @headers do |response|
+    it '#show support fetch_shared_maps_count, fetch_shared_tables_count and fetch_users' do
+      get_json api_v1_organization_groups_show_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: @group_1.id, api_key: @org_user_owner.api_key, fetch_shared_tables_count: true, fetch_shared_maps_count: true, fetch_users: true), { }, @headers do |response|
         response.status.should == 200
         response.body[:shared_tables_count].should_not be_nil
         response.body[:shared_maps_count].should_not be_nil
-        response.body[:members].should_not be_nil
+        response.body[:users].should_not be_nil
       end
     end
 
@@ -212,19 +212,19 @@ describe Carto::Api::GroupsController do
       end
     end
 
-    it '#add_members triggers group inclusion' do
+    it '#add_users triggers group inclusion' do
       group = @carto_organization.groups.first
       user = @org_user_1
 
-      Carto::Group.expects(:add_member_group_extension_query).with(anything, group.name, user.username)
+      Carto::Group.expects(:add_user_group_extension_query).with(anything, group.name, user.username)
 
-      post_json api_v1_organization_groups_add_members_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { user_id: user.id }, @headers do |response|
+      post_json api_v1_organization_groups_add_users_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { user_id: user.id }, @headers do |response|
         response.status.should == 200
         # INFO: since test doesn't actually trigger the extension we only check expectation on membership call
       end
     end
 
-    it '#remove_members triggers group exclusion' do
+    it '#remove_users triggers group exclusion' do
       group = @carto_organization.groups.first
       user = @carto_org_user_1
       group.users << user
@@ -232,29 +232,29 @@ describe Carto::Api::GroupsController do
       group.reload
       group.users.include?(user)
 
-      Carto::Group.expects(:remove_member_group_extension_query).with(anything, group.name, user.username)
+      Carto::Group.expects(:remove_user_group_extension_query).with(anything, group.name, user.username)
 
-      delete_json api_v1_organization_groups_remove_members_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key, user_id: user.id), {}, @headers do |response|
+      delete_json api_v1_organization_groups_remove_users_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key, user_id: user.id), {}, @headers do |response|
         response.status.should == 200
         # INFO: since test doesn't actually trigger the extension we only check expectation on membership call
       end
     end
 
-    it '#add_members allows batches and triggers group inclusion' do
+    it '#add_users allows batches and triggers group inclusion' do
       group = @carto_organization.groups.first
       user_1 = @org_user_1
       user_2 = @org_user_2
 
-      Carto::Group.expects(:add_member_group_extension_query).with(anything, group.name, user_1.username)
-      Carto::Group.expects(:add_member_group_extension_query).with(anything, group.name, user_2.username)
+      Carto::Group.expects(:add_user_group_extension_query).with(anything, group.name, user_1.username)
+      Carto::Group.expects(:add_user_group_extension_query).with(anything, group.name, user_2.username)
 
-      post_json api_v1_organization_groups_add_members_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { users: [ user_1.id, user_2.id ] }, @headers do |response|
+      post_json api_v1_organization_groups_add_users_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { users: [ user_1.id, user_2.id ] }, @headers do |response|
         response.status.should == 200
         # INFO: since test doesn't actually trigger the extension we only check expectation on membership call
       end
     end
 
-    it '#remove_members allows batches and triggers group exclusion' do
+    it '#remove_users allows batches and triggers group exclusion' do
       group = @carto_organization.groups.first
       user_1 = @carto_org_user_1
       user_2 = @carto_org_user_2
@@ -264,10 +264,10 @@ describe Carto::Api::GroupsController do
       group.users.include?(user_1)
       group.users.include?(user_2)
 
-      Carto::Group.expects(:remove_member_group_extension_query).with(anything, group.name, user_1.username)
-      Carto::Group.expects(:remove_member_group_extension_query).with(anything, group.name, user_2.username)
+      Carto::Group.expects(:remove_user_group_extension_query).with(anything, group.name, user_1.username)
+      Carto::Group.expects(:remove_user_group_extension_query).with(anything, group.name, user_2.username)
 
-      delete_json api_v1_organization_groups_remove_members_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { users: [ user_1.id, user_2.id ] }, @headers do |response|
+      delete_json api_v1_organization_groups_remove_users_url(user_domain: @org_user_owner.username, organization_id: @carto_organization.id, group_id: group.id, api_key: @org_user_owner.api_key), { users: [ user_1.id, user_2.id ] }, @headers do |response|
         response.status.should == 200
         # INFO: since test doesn't actually trigger the extension we only check expectation on membership call
       end