Can manually delete required End Date value in a new appointment and still save it

[ghost]

Expected behavior:
If End Date is required, it should not allow me to save the appointment after manually deleting the value in it.

Actual behavior:
I can manually delete the End Date and save the appointment.

Steps to reproduce:

1. Create a new appointment with some values EXCEPT End Date (leave it on all day).
2. Try to save it.
3. Notice that you get the warning popup, because End Date is required.
4. Input some correct value for End Date. The field is now green.
5. Edit the End Date field as if you were editing a text box. Delete all of its contents.
6. Notice that the field is still green even though it's empty.
7. You can try to save the appointment now, and it will succeed.

OS and Browser:
Lubuntu 14.10 32bit, Firefox 49.0.2

[ghost]

My thoughts on this:

• The date picker fields could be made uneditable w/o the datepicker control.
• If the event is All Day, why do we have an End Date field? All Day means the End Date is the end of Start Day, at least the way I understand it. (Later edit: to make appointments take multiple full-days, disregard this point)

[marcorosas1991]

Yeah, I tried your instructions and found the same issue. It would be incongruent to be able to delete the 'end date' after it is saved, but I wonder if some flexibility was intended. Who can we ask to make sure if it is legitimately a bug?

[ghost]

Pinging @jkleinsc @jglovier for confirmation.

[jkleinsc]

@BogdanAlexandru I just tested this and what is actually happening is that even after you clear out the date, it still keeps the original date saved on the model, but the display field is blank. You can verify this by clearing out the end date field, saving, click on return to go to previous screen and then return to edit the appointment and you will see that the end date is now there. So you can't really delete the end date, you can just temporarily blank out the date displayed. Start Date by the way does the same thing and Visit has the same issue.

As far as addressing the bug, it could be fixed by addressing the validation on start date and end date and frankly this should be pulled out into a shared mixin for both visit and appointment. If you look at the validation code on both app/models/appointment.js and app/models/visit.js you will see slightly different variations but the problem with both is that they need to check for an existing value in display_startDate and display_endDate because that is the field that contains what you see in the textbox. The date picker populates both the actual date field and the "display" field when you select a date and you can change the date by typing in a value vs clicking on the date picker, but clearing out the field doesn't update the actual date.

As far as locking down the dates once you pick them, what if you made a mistake or if you need to change an appointment?

Does all that make sense? Let me know if you have further questions.

[adeolabadmus]

@jkleinsc I attempted fixing this issue in #1002 by adding an input event listener to the date field. The event handler checks if the field is empty, and sets null on the actual date. That way, the validation still works.

It would have been really cool if the pikaday library provides an onClear callback to do stuff like this.