search

EPSCoR / ERCore

ERcore content management system to assist with NSF EPSCoR reporting
4 stars 7 forks source link

External engagement fields cannot be blank #45

Open aturling opened 8 years ago

aturling commented 8 years ago

In the process of checking our external engagements to see if the date filter wasn’t working (issue #44), I discovered that leaving any of the fields blank for # female students, # male students, etc. will prevent the external engagement from appearing on Table D (/reporting/external-engagement).

I’m not sure how external engagement counts were submitted with a couple of blank fields since all of the fields default to 0 and uploading a blank Excel spreadsheet populates the blank fields with 0. I guess this user manually deleted a 0 and left it blank? As of now there is no check to make sure none of the fields are blank. For now I just added a note to ask the participants to double-check all of the fields to make sure they’re not blank, but maybe we would want to add validation code for that.

iserna commented 8 years ago

Amy to test on her dev site. Update all fields to be required.

aturling commented 8 years ago

I tested making all of the attendee fields required, and it seems to work and not interfere with the Excel upload.

In the process of testing, though, I noticed that the validation runs whether you upload an Excel file or fill in the values manually. It works fine as long as the user sticks to one method or the other, but if a user starts to fill in the values by hand and then gives up and decides to upload the Excel file, the validation will run and show errors on the manual entries - even though the manual entries are ignored when the Excel file is uploaded. As an example, while testing, I deleted some of the default ‘0’s from the manual entries, then I uploaded an Excel file, clicked save, and got an error from Drupal that those empty fields in the manual entry section shouldn’t be blank.

It seems like, ideally, either one of two things should happen if the user uploads a file: 1) have the manual entry fields auto-update to match the Excel upload numbers (not sure if this is possible since currently the spreadsheet is processed during pre-save), or, 2) have the manual entry fieldset disappear and don’t run validate the manual entry fields.

I tried coding up option 2) with ‘#states’ but I’ve only used ‘#states’ for fields before and not fieldsets, and I couldn’t get my code to work for fieldsets.

tschet commented 7 years ago

Requiring those fields is the appropriate response. I would also set the fields default to 0 and the minimum value also to 0.