Standardize CSV import formats and fix private users CSV import with invalid file#9627
Merged
andreslucena merged 11 commits intodecidim:developfrom Oct 26, 2022
Merged
Standardize CSV import formats and fix private users CSV import with invalid file#9627andreslucena merged 11 commits intodecidim:developfrom
andreslucena merged 11 commits intodecidim:developfrom
Conversation
andreslucena
requested changes
Oct 6, 2022
Member
andreslucena
left a comment
There was a problem hiding this comment.
Just a minor change regarding the examples that we use, that I think that we should be consistent in all these forms and example files.
decidim-dev/lib/decidim/dev/assets/import_participatory_space_private_users.csv
Outdated
Show resolved
Hide resolved
decidim-dev/lib/decidim/dev/assets/import_participatory_space_private_users_invalid_col_sep.csv
Outdated
Show resolved
Hide resolved
decidim-dev/lib/decidim/dev/assets/import_participatory_space_private_users_nok.csv
Outdated
Show resolved
Hide resolved
decidim-dev/lib/decidim/dev/assets/import_participatory_space_private_users_with_bom.csv
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Member
|
Heads-up @ahukkanen, there are some failing specs. Some are related to the last set of changes. |
Contributor
Author
|
@andreslucena Specs fixed. Note that there are two unrelated a11y specs broken that should be fixed by #9929. |
entantoencuanto
added a commit
that referenced
this pull request
Oct 26, 2022
* develop: (35 commits) Install turbo-rails (#9881) Fix conference invitations (#9664) Fix invalid rendering of meeting and proposal body texts (#9764) Make documentation site work with multiple versions (#9917) Bump versions on install docs (#9916) Standardize CSV import formats and fix private users CSV import with invalid file (#9627) Fix: The i18n locales selector is showing a dropdown with 3 languages (#9902) Make Scopes field in debates translatable (#9903) Make ToS agreement translatable (#9909) Fix issues with a11y specs (#9929) Remove invitations badge (#9906) Make initiatives order translatable (#9905) Add missing active actions on admin navigation menu (#9904) Fix user sign up with invalid name (#9896) Remove duplication of LastActivity queries (#9895) Rename IgnoredMethods to AllowedMethods in Rubocop configuration (#9893) Exclude malformed file from codeclimate configuration (#9910) Fix correct resource linking for amendments (#9887) Fix superposition in admin's error forms (#9871) Add missing i18n key in Initiatives (#9892) ...
entantoencuanto
added a commit
that referenced
this pull request
Oct 31, 2022
* develop: (36 commits) Fix proposal etiquette and length validator with base64 images (#9639) Install turbo-rails (#9881) Fix conference invitations (#9664) Fix invalid rendering of meeting and proposal body texts (#9764) Make documentation site work with multiple versions (#9917) Bump versions on install docs (#9916) Standardize CSV import formats and fix private users CSV import with invalid file (#9627) Fix: The i18n locales selector is showing a dropdown with 3 languages (#9902) Make Scopes field in debates translatable (#9903) Make ToS agreement translatable (#9909) Fix issues with a11y specs (#9929) Remove invitations badge (#9906) Make initiatives order translatable (#9905) Add missing active actions on admin navigation menu (#9904) Fix user sign up with invalid name (#9896) Remove duplication of LastActivity queries (#9895) Rename IgnoredMethods to AllowedMethods in Rubocop configuration (#9893) Exclude malformed file from codeclimate configuration (#9910) Fix correct resource linking for amendments (#9887) Fix superposition in admin's error forms (#9871) ...
Quentinchampenois
pushed a commit
to Quentinchampenois/decidim
that referenced
this pull request
Nov 23, 2022
…invalid file (decidim#9627) * Standardize the CSV import file formats * Refactor the CSV import views to show examples correctly * Do not crash the import if the CSV is invalid * Test the private users CSV import with invalid column separator * Improve the error message when the CSV could not be imported * Apply suggestions from code review Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com> * Fix expectations in specs after changing the example files Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
As discussed at #8298, the CSV file formats could use some standardization so that we wouldn't have different formats in different locations of the admin panel.
At #7084 we introduced a standardized API to read the different import file formats. This PR refactors the following imports to use the readers implementation from that PR:
The problem for standardizing the other import functionality e.g. in verifications is that these APIs live now at the
adminmodule. It should be considered as a separate issue to move these under thecoremodule to also utilize them e.g. at verifications.I also left the census import at the
electionsmodule untouched because it had some additional functionality I did not want to touch in this PR for reading the CSV. But I standardized that module as well as verifications to also use the same configured CSV column separator as the rest of the imports.Another note to make is that while the import API supports also
.xlsxand.jsonformats, I did not enable them for these two imports because that would also require some other changes e.g. in the UI examples, etc. which would increase the complexity. This could be also handled as a separate issue (i.e. changing the allowed content types at the import forms to include all possible formats).This also fixes the actual issue that started the whole discussion at #8298.
📌 Related Issues
Testing
,(which is invalid)