Skip to content

Add malformed file errors when CSV reading fails#9613

Merged
andreslucena merged 8 commits intodecidim:developfrom
mainio:fix/7240
Oct 6, 2022
Merged

Add malformed file errors when CSV reading fails#9613
andreslucena merged 8 commits intodecidim:developfrom
mainio:fix/7240

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

🎩 What? Why?

At the admin panel when we upload specific CSV files and they are malformed (e.g. wrong encoding), the system will just show a general HTTP 500 error for the user which can be confusing.

This adds proper error messages in these situations.

📌 Related Issues

Testing

  • Go to the participatory space private user import from CSV view
  • Import the file added in this PR (import_participatory_space_private_users_iso8859-1.csv) as is
  • See the error on the page without an exception

📷 Screenshots

Malformed CSV error

@andreslucena
Copy link
Copy Markdown
Member

Great fix! I can confirm that's working on those forms.

I found out that I can reproduce the 500 error on Admin -> Participants -> Organization's census (/admin/csv_census/). Can you implement the fix there 🙏🏽 @ahukkanen? Thanks

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

I've left a comment with a minor change, can you check it out whenever you have time? Thanks

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

👍🏽

@andreslucena
Copy link
Copy Markdown
Member

The failing spec should be green already, as it's fixed with #9863. Can you merge with develop @ahukkanen 🙏🏽? Thanks

@andreslucena
Copy link
Copy Markdown
Member

@ahukkanen can you check my last comment 🙏🏽?

@ahukkanen
Copy link
Copy Markdown
Contributor Author

Done @andreslucena

@ahukkanen
Copy link
Copy Markdown
Contributor Author

The broken core and proposals specs are unrelated.

They will be fixed by #9877.

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽

I'll wait for @fblupi review before merging this. Let me know when this is done.

Copy link
Copy Markdown
Member

@fblupi fblupi left a comment

Choose a reason for hiding this comment

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

LGTM. It can be merged 👍

@andreslucena andreslucena merged commit ca05de4 into decidim:develop Oct 6, 2022
@ahukkanen ahukkanen deleted the fix/7240 branch October 6, 2022 07:17
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
* Add malformed file errors when CSV reading fails

* Exclude the introduced ISO 8859-1 encoded file from the i18n scan

* Fix the formatting server error for the CSV census form
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CSV uploads fail with error 500 when file has an unexpected encoding

3 participants