Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when importing a zip, csv should be import.csv #2424

Merged
merged 2 commits into from
Jul 22, 2019

Conversation

daneryl
Copy link
Collaborator

@daneryl daneryl commented Jul 15, 2019

fixes #2310

PR checklist:

  • Update READ.me ?
  • Update API documentation ?

QA checklist:

  • Smoke test the functionality described in the issue
  • Test for side effects
  • UI responsiveness
  • Cross browser testing
  • Code review

@daneryl daneryl force-pushed the 2310_import_csv_filename branch 6 times, most recently from 10f4621 to 10c32f3 Compare July 19, 2019 11:20
@habbes
Copy link
Contributor

habbes commented Jul 19, 2019

@daneryl I've tested again and now it seems to work fine.

I found the icon used for the error message to be strange, but I don't think that's a major issue
Screenshot 2019-07-19 at 15 31 12

I also tried using a file named import.csv that was actually an image and not an a csv file and it showed the following error:
Screenshot 2019-07-19 at 15 34 53

I guess that's okay as well.

And finally, I created a template with a required field and used imported a csv file that does not have that field. The import worked without reporting any issues. And the entities were create successfully, without the required field. Of course I don't think this is an issue with the import feature, I just think it means we don't have server-side validation of entities against the template schema.

If the error icon and the handling of non-csv file are note considerable issues, then this feature looks good to me.

@daneryl
Copy link
Collaborator Author

daneryl commented Jul 19, 2019

@habbes, thanks, there is an issue to work on the ui for the import csv #2353, maybe we should include the icon that you mention, anyways that a generic, the only thing i wanted to fix here is the no handling errors situation. specific entities validation etc its a whole other topic.

@habbes
Copy link
Contributor

habbes commented Jul 19, 2019

@daneryl then in that it's good enough for me.

@daneryl
Copy link
Collaborator Author

daneryl commented Jul 19, 2019

@habbes, @RafaPolit, @txau maybe a small validation to check if the csv its actually a csv ? but dont know if we can do that properly

@daneryl daneryl force-pushed the 2310_import_csv_filename branch from 10c32f3 to 62f4902 Compare July 19, 2019 15:18
@daneryl daneryl force-pushed the 2310_import_csv_filename branch from 62f4902 to 6089ab0 Compare July 22, 2019 08:09
@RafaPolit RafaPolit merged commit 1e80fc1 into development Jul 22, 2019
@RafaPolit RafaPolit deleted the 2310_import_csv_filename branch July 22, 2019 14:17
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.

Import CSV 2.0
3 participants