cockpit icon indicating copy to clipboard operation
cockpit copied to clipboard

[bug] Importing "object-structured" fields sets fields to `null`

Open abernh opened this issue 3 years ago • 2 comments

The bug

When exporting and importing a collection with "complex" field types like asset or set via the UI, those "object-structured" fields values are not imported but set to null.

Steps to reproduce

  • created a simple collection with
    • text field
    • asset field
    • set field with text sub-fields
  • add entry with some data for all (sub)fields
  • save
  • export as JSON (via UI) The values for all fields show up correctly in the export
  • imported (via UI, with auto-detected default field mapping) --> asset and set field were set to null

Note: only tested with JSON

Point of bug origin

The problem starts on doImport when the parsed data is once more re-parsed to be then sent to Import::execute. For any fields that have an object instead of a simple text value it expects a type key that is then compared with the field.type. And that type key is completely missing.

https://github.com/agentejo/cockpit/blob/568b0124352f6d27df359e8c19a70d2dd1961e87/modules/Collections/views/import/collection.php#L330

Possible fix

This particular check was added in 97bb6b6 in order to allow the importing of multiple links (says the commit message). It was later adjusted to ignore arrays (and let them pass un-checked) 7fdd136 I was not able to determine if this check for the type key was ever working, as by default no field seems currently to export a type key - also not collection-links (which are currently an array).

Options

(1) Removing the the check

.... and "trust" the mapping process that the imported value (string, object, array) is the expected one.

(2) Adding the type key into the export

The alternative is to add the missing type key.

(2A) Either by adding it to all field types and moving the value into value

Pro: consistency Con: adds a lot of boilerplate to the export making the export itself not that useful for anything else but importing.

[
  {
   _id: ...,
   textfield1: {
     type: 'text',
     value: 'this is the value' 
   },
   setfield2: {
     type: 'set',
     value: {
       subfield1: 'some more text',
       subfield2: 'and more text'
     }
   },
   asset3: {
     type: 'asset',
     value: { ... assetobj ... }
   },
   ...
]

(2B) as a "private" key _type to all fields with an object as value

Pro: less boilerplate than 2A Con: _type would be the only "meta"-key in the whole export

[
  {
   _id: ...,
   textfield1: 'this is the value',
   setfield2: {
     _type: 'set',
     subfield1: 'some more text',
     subfield2: 'and more text'
   },
   asset3: {
     _type: 'asset',
     ... assetobj ... 
   },
   ...
]

(2C) Extending the export format to be more like collections/entries/

meaning it is split in a fields and a data part, where fields will hold the type information for the to be expected values in data

{
 fields: ...,
 data: [...]
}

Pro: hardly any boilerplate or "meta" fields Con: changes export format leads to backwards compatibility issues

Suggestion

My suggestion is to go for (1) and treat object-type values like any other.

Notes

Adjustments to the export would needed to be done in the Collections\Controller\Admin->export function after the entries are retrieved and before they are pretty-jsonfied. https://github.com/agentejo/cockpit/blob/568b0124352f6d27df359e8c19a70d2dd1961e87/modules/Collections/Controller/Admin.php#L420

Adjustments to the import (2C) are in modules/Collections/assets/import/parser.js

Possible workaround (*not tested)

Disable lines 330,331 in the cockpit source if you trust your import-data-structure.

Related

https://discourse.getcockpit.com/t/import-set-field-type-not-working/2052/2

abernh avatar Aug 27 '21 08:08 abernh

My 2 cents: solution (2) won't work for an import of data not coming from a cockpit export.

I am currently trying to import a list of cities from an open data and the {"lat":, "lng":_} object does not get imported. And I don't feel like solution 2 would help me.

Ciseur avatar Nov 01 '21 17:11 Ciseur

I just tryed the suggested workaround : disabling line 330 and 331 on cockpit/modules/Collections/views/import/collection.php : it works!

Ciseur avatar Nov 01 '21 17:11 Ciseur