7018 export visualization full metadata

[juanignaciosl]

This closes #7018. Highlights:

• It uses ActiveRecord models for writes.
• Instead of using ActiveRecord for "things that you should do when you save a visualization" it calls the (old) logic at the end of the save at VisualizationsExportPersistenceService. This approach has some advantages IMHO:
  • It guarantees that it's only invoked once (see Improve importing named map saves and widget fetching #6901 about the problem of having this logic at every visualization-related model).
  • Splits external system calls (such as Maps API calls) from model logic, allowing a more precise control and handling.
• It separates the import/export concern from the persist into a user account concern. This way, multiple exporting versions will share the same persistence logic through the model object tree, which is used as a interchange structure.

This is the first step towards #6365. In the future it will be integrated in the import/export feature, but currently it can be used to import/export full visualizations as long as target user has the needed tables. Usage example:

v_id = '3bf3a072-013e-11e6-a2e1-04013fc66a01'
u1 = Carto::User.where(username: 'juanignaciosl-ded02').first
u2 = Carto::User.where(username: 'juanignaciosl-ded-02-admin').first
require_relative './app/services/carto/visualizations_export_service_2'

exported_string = Carto::VisualizationsExportService2.new.export_visualization_json_string(v_id)
imported_visualization = Carto::VisualizationsExportService2.new.build_visualization_from_json_export(exported_string)
saved_visualization = Carto::VisualizationsExportPersistenceService.new.save_import(u2, imported_visualization)

[houndci-bot]

Freeze mutable objects assigned to constants.

[javitonino]

🏃 to a meeting. Passing the baton to @gfiorav. Ping me afterwards to continue my review.

[gfiorav]

Let's make it serial: I'll CR when you've answered @javitonino 's comments 😄

[juanignaciosl]

@javisantana: take a look at the import/export spec, there's a hash with the structure of the JSON. TL;DR: a JSON with a version and a visualization with the object tree. No ids and implicit order (layers, overlays...)

@javitonino:

• This is only for derived visualizations, so I think layers_user_tables (and user_tables) are not needed.
• Custom basemaps: base layers are exported "as is". It assumes that basemap configuration is compatible among imports.

PS: reviewing your CRs I've realized that I must add derived visualizations explicit check and check that a user that hasn't private maps enabled can't import in private mode. I'll check both and ping gfiorav afterwards.

[javitonino]

@juanignaciosl

• Derived visualizations do have entries in layers_user_tables, to map the layers to the user table it takes its data from. AFAIK, this is used when deleting a dataset to check if it has depending visualizations.
• I just checked that custom basemaps are copied when adding them to a visualization, so no problem here 👍

[houndci-bot]

Assignment Branch Condition size for save_import is too high. [43.52/35]

[juanignaciosl]

@javitonino:

• I know about that metadata, but since user_tables implies a mapping to a physical table I'll wait for Export/import visualization data #7057 to consider exporting them. I'm not sure about when it's needed, since we compute the relationship with the layers dynamically through table_name and query options (if present). Convenient or not, I can't import user tables yet because the table is not being imported (yet).
• Thanks for the basemap check :-)

[juanignaciosl]

@gfiorav although maybe @javitonino will still add something I've taken into account his comments and added the privacy and derived checks, so I think you can CR.

[javitonino]

@juanignaciosl

I'm not sure about when it's needed, since we compute the relationship with the layers dynamically through table_name and query options (if present).

Exactly. But when importing, this is not being run (this is on the old layer model, in after_save)
This is not getting called from the import code, as it uses the Carto model. So the recreated visualization does not have this metadata filled. Adding a call to register_table_dependencies should fix that (as long as the table exists, which I understand is the assumption for this PR).

[javisantana]

why a new version?

[juanignaciosl]

Oh, I when I saw the notice about the breaking change I thought that we had tagged it, sorry. Changed.

[javisantana]

but this is not in 3.13.0 ?

[juanignaciosl]

I just pushed the NEWS change.

[juanignaciosl]

@javitonino ok, adding register_table_dependencies and testing again. I'm not sure whether I should add this here or not, but it worths a try.

[javisantana]

should we update this as well http://cartodb.readthedocs.org/en/latest/operations/exporting_importing_visualizations.html

[juanignaciosl]

@javisantana I'd wait for the full import/export is done, including data, don't you agree?

[houndci-bot]

Prefer detect over find.

[javisantana]

that's fine, just wanted to point that we have some doc that we might want to change

[gfiorav]

I don't have much to add. 📦 it when CI passes!

[juanignaciosl]

Ok, register_table_dependencies will link the tables if they exist and it does nothing if they don't, which seems ok now. I'll do some testing and merge this.