From e21af8ab1ed7203afe3c062d1d41217c189a933d Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 6 Feb 2020 11:46:34 +0100 Subject: [PATCH 1/4] add private_map_count to /me and count kuviz --- NEWS.md | 1 + app/controllers/carto/api/user_presenter.rb | 1 + app/models/carto/user.rb | 3 +- app/models/carto/user_service.rb | 12 +++- app/models/carto/visualization.rb | 1 + app/models/permission.rb | 3 +- app/models/quota_checker.rb | 8 +-- app/models/user.rb | 27 +++++---- app/models/user/user_decorator.rb | 5 +- .../carto/visualization_query_builder.rb | 14 +++-- spec/models/user_presenter_spec.rb | 1 + .../carto/visualization_query_builder_spec.rb | 56 +++++++++++++++++++ .../carto/api/users_controller_spec.rb | 2 + 13 files changed, 108 insertions(+), 26 deletions(-) diff --git a/NEWS.md b/NEWS.md index a4978c7179a9..3b97d6383cf9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -13,6 +13,7 @@ Development - Better error reporting for BigQuery connector ([#15383](https://github.com/CartoDB/cartodb/issues/15383)) - Fix DO subscriptions when estimated_delivery_days is NULL ([#15451](https://github.com/CartoDB/cartodb/pull/15451)) - Improve management of gcloud DO settings through API keys ([#15453](https://github.com/CartoDB/cartodb/pull/15453)) +- Add private map count to /me 4.34.0 (2020-01-28) ------------------- diff --git a/app/controllers/carto/api/user_presenter.rb b/app/controllers/carto/api/user_presenter.rb index 6f1138007af1..8c7bb16584ec 100644 --- a/app/controllers/carto/api/user_presenter.rb +++ b/app/controllers/carto/api/user_presenter.rb @@ -138,6 +138,7 @@ def data(options = {}) public_privacy_map_count: @user.public_privacy_visualization_count, link_privacy_map_count: @user.link_privacy_visualization_count, password_privacy_map_count: @user.password_privacy_visualization_count, + private_privacy_map_count: @user.private_privacy_visualization_count, owned_visualization_count: @user.owned_visualization_count, all_visualization_count: @user.all_visualization_count, visualization_count: @user.visualization_count, diff --git a/app/models/carto/user.rb b/app/models/carto/user.rb index 627c2acf96e1..81911ab600d7 100644 --- a/app/models/carto/user.rb +++ b/app/models/carto/user.rb @@ -94,7 +94,8 @@ class Carto::User < ActiveRecord::Base :database_username, :database_password, :in_database, :db_size_in_bytes, :get_api_calls, :table_count, :public_visualization_count, :all_visualization_count, :visualization_count, :owned_visualization_count, :twitter_imports_count, - :link_privacy_visualization_count, :password_privacy_visualization_count, :public_privacy_visualization_count + :link_privacy_visualization_count, :password_privacy_visualization_count, :public_privacy_visualization_count, + :private_privacy_visualization_count ] => :service attr_reader :password diff --git a/app/models/carto/user_service.rb b/app/models/carto/user_service.rb index 909863f37b14..9b6870b5e3b2 100644 --- a/app/models/carto/user_service.rb +++ b/app/models/carto/user_service.rb @@ -73,12 +73,20 @@ def password_privacy_visualization_count .count end + def private_privacy_visualization_count + return 0 unless @user.id + + Carto::VisualizationQueryBuilder.user_private_privacy_visualizations(@user) + .build + .count + end + def all_visualization_count return 0 unless @user.id Carto::VisualizationQueryBuilder.user_all_visualizations(@user) - .build - .count + .build + .count end def twitter_imports_count(options={}) diff --git a/app/models/carto/visualization.rb b/app/models/carto/visualization.rb index bb73a89cc7f0..6b589ed688de 100644 --- a/app/models/carto/visualization.rb +++ b/app/models/carto/visualization.rb @@ -41,6 +41,7 @@ class Carto::Visualization < ActiveRecord::Base TYPE_KUVIZ = 'kuviz'.freeze VALID_TYPES = [TYPE_CANONICAL, TYPE_DERIVED, TYPE_SLIDE, TYPE_REMOTE, TYPE_KUVIZ].freeze + MAP_TYPES = [TYPE_DERIVED, TYPE_KUVIZ].freeze KIND_GEOM = 'geom'.freeze KIND_RASTER = 'raster'.freeze diff --git a/app/models/permission.rb b/app/models/permission.rb index 52676422299a..da8adb46044e 100644 --- a/app/models/permission.rb +++ b/app/models/permission.rb @@ -57,8 +57,7 @@ def notify_permissions_change(permissions_changes) # be applied to a type of object. But with an array this is open # to more than one permission change at a time perm.each do |p| - if real_entity_type == CartoDB::Visualization::Member::TYPE_DERIVED || - real_entity_type == CartoDB::Visualization::Member::TYPE_KUVIZ + if Carto::Visualization::MAP_TYPES.includes?(real_entity_type) if p['action'] == 'grant' # At this moment just inform as read grant if p['type'].include?('r') diff --git a/app/models/quota_checker.rb b/app/models/quota_checker.rb index a6a43be885bb..d0ea7f1ab119 100644 --- a/app/models/quota_checker.rb +++ b/app/models/quota_checker.rb @@ -45,17 +45,13 @@ def public_map_count query_builder = Carto::VisualizationQueryBuilder.new. with_user_id(@user.id). - with_types([Carto::Visualization::TYPE_DERIVED, Carto::Visualization::TYPE_KUVIZ]). + with_types(Carto::Visualization::MAP_TYPES). with_privacy(not_private) query_builder.build.count end def private_map_count - query_builder = Carto::VisualizationQueryBuilder.new. - with_user_id(@user.id). - with_types([Carto::Visualization::TYPE_DERIVED, Carto::Visualization::TYPE_KUVIZ]). - with_privacy(Carto::Visualization::PRIVACY_PRIVATE) - query_builder.build.count + Carto::VisualizationQueryBuilder.user_private_privacy_visualizations(@user).build.count end def regular_api_key_count diff --git a/app/models/user.rb b/app/models/user.rb index 04a06dd6d8d9..b0c1ed0da048 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1538,12 +1538,12 @@ def import_count # Get the count of public visualizations def public_visualization_count - visualization_count({ - type: Carto::Visualization::TYPE_DERIVED, - privacy: Carto::Visualization::PRIVACY_PUBLIC, - exclude_shared: true, - exclude_raster: true - }) + visualization_count( + type: Carto::Visualization::MAP_TYPES, + privacy: Carto::Visualization::PRIVACY_PUBLIC, + exclude_shared: true, + exclude_raster: true + ) end def public_privacy_visualization_count @@ -1551,23 +1551,30 @@ def public_privacy_visualization_count end def link_privacy_visualization_count - visualization_count(type: Carto::Visualization::TYPE_DERIVED, + visualization_count(type: Carto::Visualization::MAP_TYPES, privacy: Carto::Visualization::PRIVACY_LINK, exclude_shared: true, exclude_raster: true) end def password_privacy_visualization_count - visualization_count(type: Carto::Visualization::TYPE_DERIVED, + visualization_count(type: Carto::Visualization::MAP_TYPES, privacy: Carto::Visualization::PRIVACY_PROTECTED, exclude_shared: true, exclude_raster: true) end + def private_privacy_visualization_count + visualization_count(type: Carto::Visualization::MAP_TYPES, + privacy: Carto::Visualization::PRIVACY_PRIVATE, + exclude_shared: true, + exclude_raster: true) + end + # Get the count of all visualizations def all_visualization_count visualization_count({ - type: Carto::Visualization::TYPE_DERIVED, + type: Carto::Visualization::MAP_TYPES, exclude_shared: false, exclude_raster: false }) @@ -1576,7 +1583,7 @@ def all_visualization_count # Get user owned visualizations def owned_visualizations_count visualization_count({ - type: Carto::Visualization::TYPE_DERIVED, + type: Carto::Visualization::MAP_TYPES, exclude_shared: true, exclude_raster: false }) diff --git a/app/models/user/user_decorator.rb b/app/models/user/user_decorator.rb index 11fe6b584415..8b085b402197 100644 --- a/app/models/user/user_decorator.rb +++ b/app/models/user/user_decorator.rb @@ -13,7 +13,9 @@ def activity(options = {}) state: state, account_type: account_type, table_count: table_count, - public_map_count: public_privacy_visualization_count + link_privacy_visualization_count + password_privacy_visualization_count, + public_map_count: public_privacy_visualization_count + link_privacy_visualization_count + + password_privacy_visualization_count, + private_map_count: private_privacy_visualization_count, map_count: all_visualization_count, map_views: map_views, geocoding_credits_count: organization_user? ? organization.get_geocoding_calls : get_geocoding_calls, @@ -52,6 +54,7 @@ def data(options = {}) public_privacy_map_count: public_privacy_visualization_count, link_privacy_map_count: link_privacy_visualization_count, password_privacy_map_count: password_privacy_visualization_count, + private_privacy_map_count: private_privacy_visualization_count, all_visualization_count: all_visualization_count, visualization_count: visualization_count, owned_visualization_count: owned_visualizations_count, diff --git a/app/queries/carto/visualization_query_builder.rb b/app/queries/carto/visualization_query_builder.rb index 822af8205d3c..e0edbf1c1805 100644 --- a/app/queries/carto/visualization_query_builder.rb +++ b/app/queries/carto/visualization_query_builder.rb @@ -17,23 +17,29 @@ def self.user_public_visualizations(user) end def self.user_public_privacy_visualizations(user) - user_public(user).with_type(Carto::Visualization::TYPE_DERIVED) + user_public(user).with_types(Carto::Visualization::MAP_TYPES) end def self.user_link_privacy_visualizations(user) new.with_user_id(user.id) - .with_type(Carto::Visualization::TYPE_DERIVED) + .with_type(Carto::Visualization::MAP_TYPES) .with_privacy(Carto::Visualization::PRIVACY_LINK) end def self.user_password_privacy_visualizations(user) new.with_user_id(user.id) - .with_type(Carto::Visualization::TYPE_DERIVED) + .with_type(Carto::Visualization::MAP_TYPES) .with_privacy(Carto::Visualization::PRIVACY_PROTECTED) end + def self.user_private_privacy_visualizations(user) + new.with_user_id(user.id) + .with_type(Carto::Visualization::MAP_TYPES) + .with_privacy(Carto::Visualization::PRIVACY_PRIVATE) + end + def self.user_all_visualizations(user) - new.with_user_id(user ? user.id : nil).with_type(Carto::Visualization::TYPE_DERIVED) + new.with_user_id(user ? user.id : nil).with_type(Carto::Visualization::MAP_TYPES) end def self.user_public(user) diff --git a/spec/models/user_presenter_spec.rb b/spec/models/user_presenter_spec.rb index 48687e302ce9..a1c4761fd248 100644 --- a/spec/models/user_presenter_spec.rb +++ b/spec/models/user_presenter_spec.rb @@ -169,6 +169,7 @@ def compare_data(original_old_data, new_data, org_user = false, mobile_sdk_enabl new_data[:public_privacy_visualization_count].should == old_data[:public_privacy_visualization_count] new_data[:link_privacy_visualization_count].should == old_data[:link_privacy_visualization_count] new_data[:password_privacy_visualization_count].should == old_data[:password_privacy_visualization_count] + new_data[:private_privacy_visualization_count].should == old_data[:private_privacy_visualization_count] new_data[:all_visualization_count].should == old_data[:all_visualization_count] new_data[:visualization_count].should == old_data[:visualization_count] new_data[:failed_import_count].should == old_data[:failed_import_count] diff --git a/spec/queries/carto/visualization_query_builder_spec.rb b/spec/queries/carto/visualization_query_builder_spec.rb index aac4a6663bea..4ac348eb4683 100644 --- a/spec/queries/carto/visualization_query_builder_spec.rb +++ b/spec/queries/carto/visualization_query_builder_spec.rb @@ -505,4 +505,60 @@ def preload_activerecord_metadata destroy_full_visualization(map, table, table_visualization, visualization) end end + + describe 'user visualizations helpers' do + before(:all) do + @user = FactoryGirl.create(:carto_user, private_maps_enabled: true) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_DERIVED, + privacy: Carto::Visualization::PRIVACY_PUBLIC) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_KUVIZ, + privacy: Carto::Visualization::PRIVACY_PUBLIC) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_CANONICAL, + privacy: Carto::Visualization::PRIVACY_PRIVATE) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_DERIVED, + privacy: Carto::Visualization::PRIVACY_PRIVATE) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_DERIVED, + privacy: Carto::Visualization::PRIVACY_LINK) + FactoryGirl.create(:carto_visualization, user_id: @user.id, type: Carto::Visualization::TYPE_DERIVED, + privacy: Carto::Visualization::PRIVACY_PROTECTED, password: 'x') + end + + after(:all) do + @user.destroy + end + + it 'returns the private maps of a user' do + result = Carto::VisualizationQueryBuilder.user_private_privacy_visualizations(@user).build + + expect(result.count).to eq 1 + expect(result.first.privacy).to eq Carto::Visualization::PRIVACY_PRIVATE + end + + it 'returns the public maps of a user' do + result = Carto::VisualizationQueryBuilder.user_public_privacy_visualizations(@user).build + + expect(result.count).to eq 2 + expect(result.all.map(&:privacy).uniq).to eq [Carto::Visualization::PRIVACY_PUBLIC] + end + + it 'returns the link maps of a user' do + result = Carto::VisualizationQueryBuilder.user_link_privacy_visualizations(@user).build + + expect(result.count).to eq 1 + expect(result.first.privacy).to eq Carto::Visualization::PRIVACY_LINK + end + + it 'returns the password maps of a user' do + result = Carto::VisualizationQueryBuilder.user_password_privacy_visualizations(@user).build + + expect(result.count).to eq 1 + expect(result.first.privacy).to eq Carto::Visualization::PRIVACY_PROTECTED + end + + it 'returns all the user maps' do + result = Carto::VisualizationQueryBuilder.user_all_visualizations(@user).build + + expect(result.count).to eq 5 + end + end end diff --git a/spec/requests/carto/api/users_controller_spec.rb b/spec/requests/carto/api/users_controller_spec.rb index 84136d649d22..77633b5c3a6e 100644 --- a/spec/requests/carto/api/users_controller_spec.rb +++ b/spec/requests/carto/api/users_controller_spec.rb @@ -19,6 +19,7 @@ user = @organization.owner carto_user = Carto::User.where(id: user.id).first FactoryGirl.create(:carto_visualization, user: carto_user, privacy: Carto::Visualization::PRIVACY_PUBLIC) + FactoryGirl.create(:carto_visualization, user: carto_user, privacy: Carto::Visualization::PRIVACY_PRIVATE) FactoryGirl.create(:carto_visualization, user: carto_user, privacy: Carto::Visualization::PRIVACY_LINK) FactoryGirl.create(:carto_visualization, user: carto_user, privacy: Carto::Visualization::PRIVACY_LINK) FactoryGirl.create(:carto_visualization, user: carto_user, @@ -79,6 +80,7 @@ expect(response.body[:user_data][:public_privacy_map_count]).to eq 1 expect(response.body[:user_data][:link_privacy_map_count]).to eq 2 expect(response.body[:user_data][:password_privacy_map_count]).to eq 3 + expect(response.body[:user_data][:private_privacy_map_count]).to eq 1 end end From 55f63f1ad7ed96b6ad94f138bb7c93254d477ea9 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 6 Feb 2020 11:53:12 +0100 Subject: [PATCH 2/4] cosmetic improvement --- app/queries/carto/visualization_query_builder.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/queries/carto/visualization_query_builder.rb b/app/queries/carto/visualization_query_builder.rb index e0edbf1c1805..ab15bdefda04 100644 --- a/app/queries/carto/visualization_query_builder.rb +++ b/app/queries/carto/visualization_query_builder.rb @@ -22,24 +22,24 @@ def self.user_public_privacy_visualizations(user) def self.user_link_privacy_visualizations(user) new.with_user_id(user.id) - .with_type(Carto::Visualization::MAP_TYPES) + .with_types(Carto::Visualization::MAP_TYPES) .with_privacy(Carto::Visualization::PRIVACY_LINK) end def self.user_password_privacy_visualizations(user) new.with_user_id(user.id) - .with_type(Carto::Visualization::MAP_TYPES) + .with_types(Carto::Visualization::MAP_TYPES) .with_privacy(Carto::Visualization::PRIVACY_PROTECTED) end def self.user_private_privacy_visualizations(user) new.with_user_id(user.id) - .with_type(Carto::Visualization::MAP_TYPES) + .with_types(Carto::Visualization::MAP_TYPES) .with_privacy(Carto::Visualization::PRIVACY_PRIVATE) end def self.user_all_visualizations(user) - new.with_user_id(user ? user.id : nil).with_type(Carto::Visualization::MAP_TYPES) + new.with_user_id(user ? user.id : nil).with_types(Carto::Visualization::MAP_TYPES) end def self.user_public(user) From 89c857a05a2dcb6a4ece749d775ffad7f90f861e Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 6 Feb 2020 11:54:16 +0100 Subject: [PATCH 3/4] news --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 3b97d6383cf9..ac701a49bff9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -13,7 +13,7 @@ Development - Better error reporting for BigQuery connector ([#15383](https://github.com/CartoDB/cartodb/issues/15383)) - Fix DO subscriptions when estimated_delivery_days is NULL ([#15451](https://github.com/CartoDB/cartodb/pull/15451)) - Improve management of gcloud DO settings through API keys ([#15453](https://github.com/CartoDB/cartodb/pull/15453)) -- Add private map count to /me +- Add private map count to /me ([#15464](https://github.com/CartoDB/cartodb/pull/15464)) 4.34.0 (2020-01-28) ------------------- From 7e00398a889fc57c6e1ea5d92c398614f71e1ea5 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 6 Feb 2020 12:48:48 +0100 Subject: [PATCH 4/4] fix typo --- app/models/permission.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/permission.rb b/app/models/permission.rb index da8adb46044e..db3b07019efb 100644 --- a/app/models/permission.rb +++ b/app/models/permission.rb @@ -57,7 +57,7 @@ def notify_permissions_change(permissions_changes) # be applied to a type of object. But with an array this is open # to more than one permission change at a time perm.each do |p| - if Carto::Visualization::MAP_TYPES.includes?(real_entity_type) + if Carto::Visualization::MAP_TYPES.include?(real_entity_type) if p['action'] == 'grant' # At this moment just inform as read grant if p['type'].include?('r')