Merge pull request #3446 from CartoDB/3404-fix-private-maps-checks

New method User#private_maps_enabled? #3404

app/models/user.rb

@@ -130,6 +130,7 @@ def before_save
       end
       self.max_layers ||= 6
       self.private_tables_enabled ||= true
+      self.private_maps_enabled ||= true
       self.sync_tables_enabled ||= true
     end
   end #before_save
@@ -807,10 +808,15 @@ def hard_twitter_datasource_limit=(val)
     self[:soft_twitter_datasource_limit] = !val
   end
 
-  def private_maps_enabled
-    enabled = super
-    return enabled if enabled.present? && enabled == true
-    /(FREE|MAGELLAN|JOHN SNOW|ACADEMY|ACADEMIC|ON HOLD)/i.match(self.account_type) ? false : true
+  def private_maps_enabled?
+    flag_enabled = self.private_maps_enabled
+    return true if flag_enabled.present? && flag_enabled == true
+
+    #TODO: remove this after making sure we have flags inline with account types
+    return true if not self.account_type.match(/FREE|MAGELLAN|JOHN SNOW|ACADEMY|ACADEMIC|ON HOLD/i)
+
+    return true if self.private_tables_enabled # Note private_tables_enabled => private_maps_enabled
+    return false
   end
 
   def import_quota

app/models/user/user_decorator.rb

@@ -52,7 +52,7 @@ def data(options = {})
         show_upgraded_message: (self.account_type.downcase != 'free' && self.upgraded_at && self.upgraded_at + 15.days > Date.today ? true : false),
         actions: {
           private_tables: self.private_tables_enabled,
-          private_maps: self.private_maps_enabled,
+          private_maps: self.private_maps_enabled?,
           dedicated_support: self.dedicated_support?,
           import_quota: self.import_quota,
           remove_logo: self.remove_logo?,

app/models/visualization/member.rb

@@ -496,7 +496,7 @@ def get_auth_tokens
       end
 
       def supports_private_maps?
-        !user.nil? && user.private_maps_enabled
+        !user.nil? && user.private_maps_enabled?
       end
 
       # @param other_vis CartoDB::Visualization::Member|nil

lib/assets/javascripts/cartodb/models/map.js

@@ -504,7 +504,7 @@ cdb.admin.Map = cdb.geo.Map.extend({
       return false;
     }
 
-    if (old) { 
+    if (old) {
       // if the layer type is the same just update the values
       if (old.get('type') === layer.get('type')) {
         var old_attribution = old.get('attribution');
@@ -762,11 +762,9 @@ cdb.admin.Map = cdb.geo.Map.extend({
             if (newLayer.wizard_properties.get('type') === 'torque' && self.layers.getTorqueLayers().length) {
               newLayer.wizard_properties.active('polygon');
             }
-            self.layers.add(newLayer);
-            // check if there was error checking if the layer is added
-            if (newLayer.collection) {
-              newLayer.save(null, opts);
-            }
+            // wait: true is used to make sure the layer is not added until confirmed it was added successfully
+            // pass opts for success/error callbacks to be triggered as expected
+            self.layers.create(newLayer, _.extend({ wait: true }, opts));
           }
 
           // Wait until the layer is totally ready in order to add it to the layers and save it

spec/models/layer_spec.rb

@@ -5,7 +5,7 @@
   before(:all) do
     @quota_in_bytes = 500.megabytes
     @table_quota = 500
-    @user = create_user(:quota_in_bytes => @quota_in_bytes, :table_quota => @table_quota)
+    @user = create_user(:quota_in_bytes => @quota_in_bytes, :table_quota => @table_quota, :private_tables_enabled => true)
   end
 
   after(:all) do

spec/models/map_spec.rb

@@ -9,7 +9,8 @@
     @table_quota    = 500
     @user           = create_user(
                         quota_in_bytes: @quota_in_bytes,
-                        table_quota:    @table_quota
+                        table_quota:    @table_quota,
+                        private_tables_enabled: true
                       )
   end
 

spec/models/named_maps_spec.rb

@@ -43,7 +43,7 @@ def named_map_url(template_id, user_api_key)
     Visualization.repository = DataRepository::Backend::Sequel.new(Rails::Sequel.connection, :visualizations)
 
     puts "\n[rspec][#{SPEC_NAME}] Creating test user database..."
-    @user = create_user( :quota_in_bytes => 524288000, :table_quota => 100 )
+    @user = create_user( :quota_in_bytes => 524288000, :table_quota => 100, :private_tables_enabled => true )
 
     puts "[rspec][#{SPEC_NAME}] Running..."
   end

spec/models/user_spec.rb

@@ -475,26 +475,31 @@
     end
   end
 
-  describe '#private_maps_enabled' do
+  describe '#private_maps_enabled?' do
     it 'should not have private maps enabled by default' do
       user_missing_private_maps = create_user :email => 'user_mpm@example.com',  :username => 'usermpm',  :password => 'usermpm'
-      user_missing_private_maps.private_maps_enabled.should eq false
+      user_missing_private_maps.private_maps_enabled?.should eq false
     end
 
     it 'should have private maps if enabled' do
       user_with_private_maps = create_user :email => 'user_wpm@example.com',  :username => 'userwpm',  :password => 'userwpm', :private_maps_enabled => true
-      user_with_private_maps.private_maps_enabled.should eq true
+      user_with_private_maps.private_maps_enabled?.should eq true
     end
 
     it 'should not have private maps if disabled' do
       user_without_private_maps = create_user :email => 'user_opm@example.com',  :username => 'useropm',  :password => 'useropm', :private_maps_enabled => false
-      user_without_private_maps.private_maps_enabled.should eq false
+      user_without_private_maps.private_maps_enabled?.should eq false
+    end
+
+    it 'should have private maps if he has private_tables_enabled, even if disabled' do
+      user_without_private_maps = create_user :email => 'user_opm3@example.com',  :username => 'useropm3',  :password => 'useropm3', :private_maps_enabled => false, :private_tables_enabled => true
+      user_without_private_maps.private_maps_enabled?.should eq true
     end
 
     it 'should have private maps if he is AMBASSADOR even if disabled' do
       user_without_private_maps = create_user :email => 'user_opm2@example.com',  :username => 'useropm2',  :password => 'useropm2', :private_maps_enabled => false
       user_without_private_maps.stubs(:account_type).returns('AMBASSADOR')
-      user_without_private_maps.private_maps_enabled.should eq true
+      user_without_private_maps.private_maps_enabled?.should eq true
     end
 
   end

spec/models/visualization/collection_spec.rb

@@ -401,11 +401,10 @@ def restore_vis_backend_to_normal_table_so_relator_works
     it 'counts total liked' do
       restore_vis_backend_to_normal_table_so_relator_works
 
-      user1 = create_user(:quota_in_bytes => 524288000, :table_quota => 500)
+      user1 = create_user(:quota_in_bytes => 524288000, :table_quota => 500, :private_tables_enabled => true)
       user1.stubs(:organization).returns(nil)
 
-      user2 = create_user(:quota_in_bytes => 524288000, :table_quota => 500)
-      user2.stubs(:private_tables_enabled).returns(true)
+      user2 = create_user(:quota_in_bytes => 524288000, :table_quota => 500, :private_tables_enabled => true)
       user2.stubs(:organization).returns(nil)
 
       table11 = create_table(user1)
@@ -508,9 +507,9 @@ def restore_vis_backend_to_normal_table_so_relator_works
     it "checks filtering by 'liked' " do
       restore_vis_backend_to_normal_table_so_relator_works
 
-      user = create_user(:quota_in_bytes => 524288000, :table_quota => 500)
-      user2 = create_user(:quota_in_bytes => 524288000, :table_quota => 500)
-      user3 = create_user(:quota_in_bytes => 524288000, :table_quota => 500)
+      user = create_user(:quota_in_bytes => 524288000, :table_quota => 500, :private_tables_enabled => true)
+      user2 = create_user(:quota_in_bytes => 524288000, :table_quota => 500, :private_tables_enabled => true)
+      user3 = create_user(:quota_in_bytes => 524288000, :table_quota => 500, :private_tables_enabled => true)
       CartoDB::Visualization::Relator.any_instance.stubs(:user).returns(user)
 
       table1 = Table.new

spec/models/visualization/member_spec.rb

@@ -428,7 +428,7 @@
 
 
       # Private maps allowed
-      @user_mock.stubs(:private_maps_enabled).returns(true)
+      @user_mock.stubs(:private_maps_enabled?).returns(true)
 
       # Forces internal "dirty" flag
       visualization.privacy = Visualization::Member::PRIVACY_PUBLIC
@@ -439,7 +439,7 @@
       # -------------
 
       # No private maps allowed
-      @user_mock.stubs(:private_maps_enabled).returns(false)
+      @user_mock.stubs(:private_maps_enabled?).returns(false)
 
       visualization.privacy = Visualization::Member::PRIVACY_PUBLIC
       visualization.privacy = Visualization::Member::PRIVACY_PRIVATE
@@ -475,7 +475,7 @@
           type:     Visualization::Member::TYPE_CANONICAL,
           user_id:  user_id
       )
-      @user_mock.stubs(:private_maps_enabled).returns(true)
+      @user_mock.stubs(:private_maps_enabled?).returns(true)
 
       # Careful, do a user mock after touching user_data as it does some checks about user too
       user_mock = mock

spec/requests/admin/visualizations_spec.rb

@@ -22,7 +22,8 @@ def app
     @user = create_user(
       username: 'test',
       email:    'test@test.com',
-      password: 'test12'
+      password: 'test12',
+      private_tables_enabled: true
     )
     @api_key = @user.api_key
     @user.stubs(:should_load_common_data?).returns(false)

spec/requests/api/tables_spec.rb

@@ -6,7 +6,7 @@
 
   before(:all) do
     CartoDB::Varnish.any_instance.stubs(:send_command).returns(true)
-    @user = create_user(:username => 'test', :email => "client@example.com", :password => "clientex")
+    @user = create_user(:username => 'test', :email => "client@example.com", :password => "clientex", :private_tables_enabled => true)
     @another_user = create_user
   end
 

spec/support/factories/users.rb

@@ -44,7 +44,7 @@ def new_user(attributes = {})
       user.password              = attributes[:password] || user.email.split('@').first
       user.password_confirmation = user.password
       user.admin                 = attributes[:admin] == false ? false : true
-      user.private_tables_enabled = attributes[:private_tables_enabled] == false ? false : true
+      user.private_tables_enabled = attributes[:private_tables_enabled] == true ? true : false
       user.private_maps_enabled  = attributes[:private_maps_enabled] == true ? true : false
       user.enabled               = attributes[:enabled] == false ? false : true
       user.table_quota           = attributes[:table_quota]     if attributes[:table_quota]