Merge pull request #8131 from CartoDB/revert-8045-7657-duplicate_maps…

…_with_analyses

Revert "Copy visualizations using export/import"

Makefile

@@ -46,6 +46,7 @@ WORKING_SPECS_1 = \
 	spec/models/map_spec.rb \
 	spec/models/map/copier_spec.rb \
 	spec/models/visualization/collection_spec.rb \
+	spec/models/visualization/copier_spec.rb \
 	spec/models/visualization/locator_spec.rb \
 	spec/models/visualization/member_spec.rb \
 	spec/models/visualization/name_checker_spec.rb \

app/controllers/api/json/visualizations_controller.rb

@@ -4,13 +4,13 @@
 require_relative '../../../models/visualization/collection'
 require_relative '../../../models/visualization/presenter'
 require_relative '../../../models/visualization/locator'
+require_relative '../../../models/visualization/copier'
 require_relative '../../../models/visualization/name_generator'
 require_relative '../../../models/visualization/table_blender'
 require_relative '../../../models/visualization/watcher'
 require_relative '../../../models/map/presenter'
 require_relative '../../../../lib/static_maps_url_helper'
 require_relative '../../../../lib/cartodb/event_tracker'
-require_dependency 'carto/visualizations_export_service_2'
 
 class Api::Json::VisualizationsController < Api::ApplicationController
   include CartoDB
@@ -44,21 +44,24 @@ def create
         current_user_id = current_user.id
 
         vis = if params[:source_visualization_id]
-                # Copy visualization
-                user = Carto::User.find(current_user_id)
-                source = Carto::Visualization.where(id: params[:source_visualization_id]).first
-                return head(403) unless source && source.is_viewable_by_user?(user) && source.derived?
-
-                visualization_copy_id = @stats_aggregator.timing('copy') do
-                  export_service = Carto::VisualizationsExportService2.new
-                  visualization_hash = export_service.export_visualization_json_hash(source, user)
-                  visualization_copy = export_service.build_visualization_from_hash_export(visualization_hash)
-                  visualization_copy.name = name_candidate
-                  Carto::VisualizationsExportPersistenceService.new.save_import(user, visualization_copy)
-                  visualization_copy.id
+                source, = @stats_aggregator.timing('locate') do
+                  locator.get(params.fetch(:source_visualization_id), subdomain)
                 end
 
-                CartoDB::Visualization::Member.new(id: visualization_copy_id).fetch
+                return head(403) unless source && source.can_copy?(current_user)
+
+                copy_overlays = params.fetch(:copy_overlays, true)
+                copy_layers = params.fetch(:copy_layers, true)
+
+                additional_fields = {
+                  type:       params.fetch(:type, Visualization::Member::TYPE_DERIVED),
+                  parent_id:  params.fetch(:parent_id, nil)
+                }
+
+                @stats_aggregator.timing('copy') do
+                  Visualization::Copier.new(current_user, source, name_candidate)
+                                       .copy(copy_overlays, copy_layers, additional_fields)
+                end
               elsif param_tables
                 viewed_user = ::User.find(username: subdomain)
 

app/models/visualization/copier.rb

@@ -0,0 +1,59 @@
+# encoding: utf-8
+require 'ostruct'
+require_relative './name_generator'
+require_relative '../map/copier'
+
+module CartoDB
+  module Visualization
+    # Creates a new visualization using another as source.
+    # Do NOT use this to create derived visualizations as creates a new map.
+    class Copier
+      def initialize(user, visualization, name = nil)
+        @user           = user
+        @visualization  = visualization
+        @name           = name
+      end
+
+      def copy(overlays = true, layers = true, additional_fields = {})
+        member = Member.new(
+          name:         new_name,
+          tags:         visualization.tags,
+          description:  visualization.description,
+          type:         type_from(additional_fields),
+          parent_id:    additional_fields.fetch(:parent_id, nil),
+          map_id:       map_copy(layers).id,
+          privacy:      visualization.privacy,
+          user_id:      @user.id
+        )
+        if overlays
+          overlays_copy(member)
+        end
+        member
+      end
+
+      private
+
+      attr_reader :visualization, :user, :name
+
+      def type_from(fields)
+        fields.fetch(:type, Member::TYPE_DERIVED)
+      end
+
+      def overlays_copy(new_visualization)
+        visualization.overlays.each.map do |overlay|
+          new_overlay = overlay.dup
+          new_overlay.visualization_id = new_visualization.id
+          new_overlay.save
+        end
+      end
+
+      def map_copy(layers)
+        @map_copy ||= CartoDB::Map::Copier.new.copy(visualization.map, layers)
+      end
+
+      def new_name
+        @new_name ||= NameGenerator.new(user).name(name || visualization.name)
+      end
+    end
+  end
+end

spec/factories/visualizations.rb

@@ -4,15 +4,14 @@
 include Carto::UUIDHelper
 
 FactoryGirl.define do
+
   factory :derived_visualization, class: CartoDB::Visualization::Member do
-    to_create(&:store)
     type 'derived'
     name "visualization #{random_uuid}"
     privacy 'public'
   end
 
   factory :table_visualization, class: CartoDB::Visualization::Member do
-    to_create(&:store)
     type 'table'
     name "visualization_#{random_uuid}"
     privacy 'public'

spec/models/layer_spec.rb

@@ -274,13 +274,20 @@
 
   describe '#uses_private_tables?' do
     it 'returns true if any of the affected tables is private' do
-      @table.table_visualization.layers(:cartodb).length.should == 1
-      @table.table_visualization.layers(:cartodb).first.uses_private_tables?.should be_true
+      Carto::NamedMaps::Api.any_instance.stubs(get: nil, create: true, update: true)
+
+      map = Map.create(user_id: @user.id, table_id: @table.id)
+      source = @table.table_visualization
+      derived = CartoDB::Visualization::Copier.new(@user, source).copy
+      derived.store
+
+      derived.layers(:cartodb).length.should == 1
+      derived.layers(:cartodb).first.uses_private_tables?.should be_true
       @table.privacy = UserTable::PRIVACY_PUBLIC
       @table.save
       @user.reload
 
-      @table.table_visualization.layers(:cartodb).first.uses_private_tables?.should be_false
+      derived.layers(:cartodb).first.uses_private_tables?.should be_false
     end
   end
 

spec/models/map_spec.rb

@@ -335,29 +335,31 @@
       @table2.user_id = @user.id
       @table2.save
 
-      visualization = @table1.table_visualization
+      source  = @table1.table_visualization
+      derived = CartoDB::Visualization::Copier.new(@user, source).copy
+      derived.store
 
-      visualization.layers(:cartodb).length.should eq 1
+      derived.layers(:cartodb).length.should eq 1
       @table1.privacy = UserTable::PRIVACY_PUBLIC
       @table1.save
-      visualization.privacy = CartoDB::Visualization::Member::PRIVACY_PUBLIC
-      visualization.store
+      derived.privacy = CartoDB::Visualization::Member::PRIVACY_PUBLIC
+      derived.store
 
-      visualization.fetch.private?.should be_false
+      derived.fetch.private?.should be_false
 
       layer = Layer.create(
         kind:     'carto',
         options:  { table_name: @table2.name }
       )
-      layer.add_map(visualization.map)
+      layer.add_map(derived.map)
       layer.save
       layer.reload
       @user.reload
 
       layer.uses_private_tables?.should be_true
 
-      visualization.map.process_privacy_in(layer)
-      visualization.fetch.private?.should be_true
+      derived.map.process_privacy_in(layer)
+      derived.fetch.private?.should be_true
     end
   end
 

spec/models/table_spec.rb

@@ -357,10 +357,9 @@ def @data_import.data_source=(filepath)
       table = create_table(user_id: @user.id)
       table.should be_private
       table.table_visualization.should be_private
-
-      map = CartoDB::Visualization::TableBlender.new(@user, [table]).blend
-      derived_vis = FactoryGirl.create(:derived_visualization, user_id: @user.id, map_id: map.id,
-                                                               privacy: CartoDB::Visualization::Member::PRIVACY_PRIVATE)
+      derived_vis = CartoDB::Visualization::Copier.new(
+        @user, table.table_visualization
+      ).copy
 
       bypass_named_maps
       derived_vis.store
@@ -369,16 +368,16 @@ def @data_import.data_source=(filepath)
       table.privacy = UserTable::PRIVACY_PUBLIC
       table.save
 
-      table.affected_visualizations.map do |vis|
+      table.affected_visualizations.map { |vis|
         vis.public?.should == vis.table?
-      end
+      }
 
       table.privacy = UserTable::PRIVACY_PRIVATE
       table.save
 
-      table.affected_visualizations.map do |vis|
+      table.affected_visualizations.map { |vis|
         vis.private?.should == true
-      end
+      }
     end
 
     it "doesn't propagates changes to affected visualizations if privacy set to public with link" do
@@ -389,8 +388,9 @@ def @data_import.data_source=(filepath)
 
       table.privacy = UserTable::PRIVACY_PUBLIC
       table.save
-      map = CartoDB::Visualization::TableBlender.new(@user, [table]).blend
-      derived_vis = FactoryGirl.create(:derived_visualization, user_id: @user.id, map_id: map.id)
+      derived_vis = CartoDB::Visualization::Copier.new(
+          @user, table.table_visualization
+      ).copy
 
       bypass_named_maps
       derived_vis.store
@@ -400,12 +400,12 @@ def @data_import.data_source=(filepath)
       table.save
       table.reload
 
-      table.affected_visualizations.map do |vis|
-        vis.public?.should eq vis.derived?         # Derived kept public
-        vis.private?.should eq false               # None changed to private
-        vis.password_protected?.should eq false    # None changed to password protected
-        vis.public_with_link?.should eq vis.table? # Table/canonical changed
-      end
+      table.affected_visualizations.map { |vis|
+        vis.public?.should == vis.derived?  # Derived kept public
+        vis.private?.should == false  # None changed to private
+        vis.password_protected?.should == false  # None changed to password protected
+        vis.public_with_link?.should == vis.table?  # Table/canonical changed
+      }
     end
 
     it 'receives privacy changes from the associated visualization' do
@@ -771,11 +771,12 @@ def @data_import.data_source=(filepath)
 
     it 'deletes derived visualizations that depend on this table' do
       bypass_named_maps
-      table = create_table(name: 'bogus_name', user_id: @user.id)
-
-      map = CartoDB::Visualization::TableBlender.new(@user, [table]).blend
-      derived = FactoryGirl.create(:derived_visualization, user_id: @user.id, map_id: map.id)
+      table   = create_table(name: 'bogus_name', user_id: @user.id)
+      source  = table.table_visualization
+      derived = CartoDB::Visualization::Copier.new(@user, source).copy
+      derived.store
 
+      rehydrated = CartoDB::Visualization::Member.new(id: derived.id).fetch
       table.reload
 
       table.destroy
@@ -2268,9 +2269,9 @@ def @data_import.data_source=(filepath)
       table.should be_private
 
       Carto::NamedMaps::Api.any_instance.stubs(get: nil, create: true, update: true)
-
-      map = CartoDB::Visualization::TableBlender.new(@user, [table]).blend
-      derived = FactoryGirl.create(:derived_visualization, user_id: @user.id, map_id: map.id)
+      source  = table.table_visualization
+      derived = CartoDB::Visualization::Copier.new(@user, source).copy
+      derived.store
       derived.type.should eq(CartoDB::Visualization::Member::TYPE_DERIVED)
 
       # Do not create all member objects anew to be able to set expectations
@@ -2293,8 +2294,9 @@ def @data_import.data_source=(filepath)
       table.save
 
       Carto::NamedMaps::Api.any_instance.stubs(get: nil, create: true, update: true)
-      map = CartoDB::Visualization::TableBlender.new(@user, [table]).blend
-      derived = FactoryGirl.create(:derived_visualization, user_id: @user.id, map_id: map.id)
+      source = table.table_visualization
+      derived = CartoDB::Visualization::Copier.new(@user, source).copy
+      derived.store
       derived.type.should eq(CartoDB::Visualization::Member::TYPE_DERIVED)
 
       # Scenario 1: Fail at map saving (can happen due to Map handlers)
@@ -2303,7 +2305,9 @@ def @data_import.data_source=(filepath)
 
       ::Map.any_instance.stubs(:save).once.raises(StandardError)
 
-      expect { table.save }.to raise_exception StandardError
+      expect do
+        table.save
+      end.to raise_exception StandardError
 
       table.reload.privacy.should eq UserTable::PRIVACY_PUBLIC