Skip to content

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Aug 20, 2021

A better explanation of the bug is if you import fields with names that contain non-ascii characters, then those will always import as strings, no matter what you select in the dropdown.


Uncommenting this debug call it prints {“keyPathToTransform”:{“”:“Number”},“keyPath”:“价格“}. See that it made the path a blank string. It clearly stripped all non-ascii characters when it tried to turn it into dot notation.

That happened here

Here you can see where that change was introduced: https://github.com/mongodb-js/compass-import-export/pull/18/files#diff-66ee9e953feb4b9aaa484b3de0e43691c40b17b124bc2c4a15abf0b5ffdc363bR61-R63

Then questioned with no explanation given, then just merged anyway:

Screenshot 2021-08-20 at 13 35 02

Then eventually the TODO message just got deleted, again with no explanation given. Somewhere in this PR 45cd5bf#diff-517194336722d4801481b2c8225a2ba2d2491f2f5e42a59ad1497692589f4119

Mongodb has no issue with chinese characters in the field names. I confirmed this by importing the data using mongoimport.

This PR removes the line that strips all that and then that fixes the problem. I can now import that CSV file using compass and it keeps and uses the field types you manually assign.

So.. I don't know. Go with this until something breaks again and then next time properly document it, explain why and add tests?

# Verdaccio storage path
storage
storage
.ackrc
Copy link
Contributor Author

@lerouxb lerouxb Aug 20, 2021

Choose a reason for hiding this comment

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

I had to add an .ackrc to the compass folder so I can use ack to search for code and ignore all the giant packages/*/lib/index.js files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would have been nice if those were .min.js so it would automagically be ignored. Or so it would be easy to write an accurate ignore pattern.

Copy link
Collaborator

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Yeah, I agree that the regexp is way too strict in any case. Let’s remove it and see if anybody complains.

@lerouxb lerouxb merged commit a25e0eb into main Aug 23, 2021
@lerouxb lerouxb deleted the COMPASS-4987-chinese branch August 23, 2021 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants