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

CSV export with all submission fields included fails when field names have a dot (.) or comma #1128

Open
6 tasks
GuySartorelli opened this issue Dec 12, 2021 · 4 comments

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented Dec 12, 2021

If a custom field name has a period (aka a dot aka .) in it, and that field is included in a csv export, it results in a LogicException. For example, a field with the name A.field results in

Uncaught Exception LogicException: "A is not a relation/field on SilverStripe\UserForms\Model\Submission\SubmittedForm" at /home/webspace/te-tumu-paeroa-uat/site/vendor/silverstripe/framework/src/ORM/DataObject.php line 3225

I know that it's already protected against users adding a period through the edit form, so this is a pretty low priority, but an Extension class that implements onBeforeWrite to set the name based on the title might accidentally include a period, which could go unnoticed until someone tries to export the CSV.
It would be better to catch this when writing the record instead (e.g. by implementing validate on EditableFormField) to explicitly disallow periods.

ACs

  • (assuming we go ahead with this, this could be considered a breaking change)
  • Add validation to disallow periods or commas on the Name field
  • Possibly remove whatever is currently preventing periods being added in the edit form (have asked about this below)
  • Ensure front-end error messaging on validation errors remains working
  • Configuration option for disabling validation is available e.g. private static disable_name_period_validation = false for any sites with existing data. Deprecate this config value straight away.
  • Changelog is updated to highlight change
@emteknetnz
Copy link
Member

@GuySartorelli could you help me understand this one a bit better. I'm not sure where I know that it's already protected against users adding a period through the edit form, is coming from, I've had a quick look through EditableFormField.php though I couldn't find out

@emteknetnz
Copy link
Member

I'm on the fence with this one, this one could be considered a breaking change

@GuySartorelli
Copy link
Member Author

I wrote this issue a while ago. I'm not sure what I meant either - I can't seem to reproduce the protected against users adding a period through the edit form behaviour I mentioned.

@sig-peggy
Copy link

A similar error occurs when there's a comma in the field name

@GuySartorelli GuySartorelli changed the title CSV export with all submission fields included fails when field names have a dot (.) CSV export with all submission fields included fails when field names have a dot (.) or comma Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants