Feat: mapeoProject.importConfig

[tomasciccola]

This should close #401

• create implementation of importConfig
• create zip file with default configuration (grabbed from here
• add tests

[tomasciccola]

should I also delete every field and icon? or does it makes more sense to get the {iconName, iconId} and {fieldName, fieldId} so that when loading presets we directly reference those?

[tomasciccola]

For now I implemented the first case (basically delete every preset, field and doc). Matching icons of fields against names doesn't seem to be super reliable...

[gmaclennan]

Yes I would delete every field. For icons it would be nice to re-use them somehow (e.g. checking hashes) but for now let's just delete them all.

[tomasciccola]

There are some files on the config that I don't know what to do with:

• icons.json -> has size, placement (I think) and pixelRatio
• icons.{png,svg} -> I don't think we need this
• metadata.json -> I don't think we need this
• translations.json -> spanish names for fields
• style.css
• VERSION -> maybe we need it to have a runtime check (fail if version is older than new config format)

[tomasciccola]

All the deletion of fields, icons and presets prior to importing the config is commented until #420 and #421 gets merged

[gmaclennan]

Good work on this. There were a few bugs and I got a bit carried away reviewing and fixing, and ended up with with a bit of a restructure which is here: 17e34e0

I haven't tested this, but hopefully can serve as guidance. It creates a readConfig abstraction, so that we don't end up with a huge method in project.importConfig - we can keep the file parsing separate to actually adding the presets to the database.

There were a few bugs that I addressed:

• (not exactly a bug): the code was reading all icons into memory (because they were all added to the icons object), which would be nice to avoid because this could take a lot of memory.
• field.key is not the same as the id that is referenced in preset.fields. preset.fields refers to Object.keys(presetsFile.fields).
• It's important to validate all the data we import, because it could actually be anything. I've added validation functions, and I avoid importing extra properties that we don't know about (by reading from the schema, rather than just spreading the object from the config file)

I've tried to be really strict about validating everything, but I don't throw on everything - I thought it was better to try to import what we can.

Things that throw

• No presets prop
• No fields prop
• A preset names an icon but it doesn't exist
• A preset names a field but it doesn't exist
• Not being able to read the config file at all

Things that do not throw

• Invalid preset, field or icon (although it may cause one of the errors above if the field or icon is referenced)

I'm not sure what to do with these errors / warnings and they aren't returned to the user right now. I think it's a question for the design team about how to present a warning like "We could import the presets but there were errors". Maybe they will decide to throw on any error.

I'll answer your other questions in a comment with inline replies

[gmaclennan]

There are some files on the config that I don't know what to do with:

Good point, I had forgotten about these.

    icons.json -> has size, placement (I think) and pixelRatio
    icons.{png,svg} -> I don't think we need this

We are not yet using these. These are the sprite sheets that Mapbox uses for rendering icons on the map (the json references the icon location within the sprite sheet icons.png). The goal was to render the map with icons for observations, but we can ignore this data for now until we add that feature (and maybe generate the sprite sheet at runtime if we need it)

    metadata.json -> I don't think we need this

Yes I think it's ok to ignore for now. A question for the design team if we want this to update the project name and/or description, or any other project settings. Could be both useful and dangerous (e.g. could be confusing to see project settings changed when importing a config file)

    translations.json -> spanish names for fields

Hmmm yes I had not thought about this. Can you maybe have a think about how we might modify the schema to include this in the field / preset records we store in the database? I don't think it makes sense to store it as a separate data type?

    style.css

This was used in ID Editor (Mapeo territory view) to style certain features on the map in a particular way (we only used it in Waorani mapping as far as I know). I think it's ok to skip this for now.

    VERSION -> maybe we need it to have a runtime check (fail if version is older than new config format)

Yes, I think we should throw an error if either an older or a newer version.

[tomasciccola]

Good work on this. There were a few bugs and I got a bit carried away reviewing and fixing, and ended up with with a bit of a restructure which is here: 17e34e0

Thanks for this! I'll take a look at it. Yeah, I knew I was doing an inefficient way of reading icons (mostly because I needed to group variants/aggregate them by icon name) but I didn't want to mess with generators; I was basically going with the most straight away implementation for starters...

Do you think all the deletion stuff should also live on the importConfig function (as a returned method)??

[tomasciccola]

does it make sense to do this here instead of moving it outside to a function in src/config-imprort.js? I left it here since we are using a couple of methods of the class and it felt weird passing them to the function as params...

[gmaclennan]

I'm fine with it here. Could also add a deleteAll() method to DataType or we could do a quick helper:

async function deleteAll(dataType) {
  const deletions = []
  for (const doc of await dataType.getMany()) {
    deletions.push(dataType.delete(doc.versionId))
  }
  return Promise.all(deletions)
}

[EvanHahn]

LGTM. Thanks for dealing with all my nitpicks!

[tomasciccola]

before merging, I've created an issue here to see if it makes sense implementing dataType.deleteAll()