Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Conversation

imlucas
Copy link
Contributor

@imlucas imlucas commented Nov 18, 2019

Description

Adds a preview table of the file you are importing with checkboxes to exclude fields and select to specify a different type.

Motivation and Context

Using Import to Reshape Apple Health Data Export

Default to CSV:

Screenshot 2019-11-21 00 03 28

Tweak delimiter so it looks right:

Screenshot 2019-11-21 00 03 51

Specified field transforms:

const fields = [
  { path: 'type', checked: false, type: 'String' },
  { path: 'sourceName', checked: true, type: 'String' },
  { path: 'sourceVersion', checked: true, type: 'Number' },
  { path: 'creationDate', checked: true, type: 'Date' },
  { path: 'startDate', checked: true, type: 'Date' },
  { path: 'endDate', checked: true, type: 'Date' }
];

Import done:
Screenshot 2019-11-21 00 15 24

CRUD has my enriched types:

And the schema view is super pretty:

Screenshot 2019-11-21 00 23 52

Types of changes

  • Minor (non-breaking change which adds functionality)

Open questions

  • Change to utils/import/*.js instead of utils/import-*.js?
  • The preview could have partial objects if spill over into next buffer. Can we back pressure against the input source for real instead of this hacky impl?
  • move EJSON.deserialize() from parsers to apply-import-type-and-projection?
  • better name for apply-import-type-and-projection?

@imlucas imlucas changed the title Preview table with project and type customization COMPASS-3826 COMPASS-3947: Import preview table with project and type customization Nov 26, 2019
@imlucas imlucas marked this pull request as ready for review November 26, 2019 14:51
@imlucas imlucas requested review from durran and lrlna and removed request for durran November 26, 2019 14:51
Copy link
Contributor

@lrlna lrlna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments on the UX side of things!

In response to your comments from above:

Change to utils/import/.js instead of utils/import-.js?

I like the import-*.js, but that could be just be me!

The preview could have partial objects if spill over into next buffer. Can we back pressure against the input source for real instead of this hacky impl?

Replied in comments! tl;dr: i think peek-stream is meant to make sure we don't deal with partial objects.

move EJSON.deserialize() from parsers to apply-import-type-and-projection?

Not sure which EJSON.deserialize() you mean? There is a comment for progressStream in the parsers file, but I think it makes sense to have that with the parsers. Since when you parse, you have progress.

better name for apply-import-type-and-projection?

I think with the style of import-*.js style, would be good to have it as import-apply-type-and-projection. But otherwise it's nice and descriptive and I right away understand what's going on.

@imlucas imlucas self-assigned this Dec 2, 2019
@imlucas imlucas mentioned this pull request Dec 4, 2019
imlucas added a commit that referenced this pull request Dec 4, 2019
Cutting master so all of your export improvements from the past few days can be upstreamed into compass asap with @mongodb-js/compass-import-export@^4.2.0.

The import type casting stuff in #18 is complex enough that I don't want it blocking so it will go as 5.0.0. We can keep this 4.x-releases branch ref around just in case casting performance is a real dud and we want to take another stab at it.

Now that this release has been published, a PR to mongodb-js/compass 📦 @mongodb-js/compass-import-export@^4.2.0 has changelog:

UX improvements: display doc count, do not display line numbers #20
COMPASS-3961: allow adding missing fields #19
COMPASS-3851: ability to add fields when exporting collection #17
@imlucas imlucas merged commit 22e0fec into master Dec 4, 2019
@imlucas imlucas deleted the preview-table branch December 4, 2019 01:19
imlucas added a commit to mongodb-js/compass that referenced this pull request Dec 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants