TEAMMATES / teammates

This is the project website for the TEAMMATES feedback management tool for education
https://teammatesv4.appspot.com/
GNU General Public License v2.0
1.66k stars 3.3k forks source link

Instructor: edit session: visible date: reject incorrect dates #8791

Closed damithc closed 5 years ago

damithc commented 6 years ago

v6.5.1

Steps:

  1. Login as instructor

  2. Edit visible date of an existing session by modifying the text box (not the date picker)

  3. Enter an incorrect date where the day doesn't match the date e.g., Take a valid date and change the date but not the day -- Apr 19th is not a Tuesday image

  4. Save

Expected: The date is rejected or auto-corrected upon save Actual: The incorrect date is saved

shaharyar-shamshi commented 6 years ago

Sit I am working on this by adopting the appraoch of autocorrection

whipermr5 commented 6 years ago

If the page is reloaded, a valid date should show up. The field is just not getting updated upon submit because it happens over AJAX. @shaharyar-shamshi a good way to solve this would be to ride on the existing resolved time fields in the JSON response, introduced in #8687. The frontend code for updating the fields dynamically upon submit is already written (originally meant for ambiguous date times).

shaharyar-shamshi commented 6 years ago

@whipermr5 sir actually I am introduction a the method in Timehelper to match the day and date if they are wrong then I am changing the day to match it with the date

tanhengyeow commented 6 years ago

I investigated on this issue and the problem lies with invalid datetime (from user input) being parsed to null upon conversion to LocalDateTime format at the backend, thus not further processed.

As mentioned by @whipermr5 , the update to the frontend happens over AJAX thus a page refresh would not persist the invalid datetime and would show the date back to the original datetime before modification.

I was thinking of reflecting the original datetime back in the field upon submit and making the status message more verbose to include "...... Invalid datetime fields are reset to the original datetime field."

@damithc Your thoughts on this?

damithc commented 6 years ago

I was thinking of reflecting the original datetime back in the field upon submit and making the status message more verbose to include "...... Invalid datetime fields are reset to the original datetime field."

Sounds good @tanhengyeow It's good if we can also indicate which exact field was reverted

tanhengyeow commented 6 years ago

@damithc How's the proposed changes?

Success scenario (same as before): success

Invalid scenario: error

Invalid scenario screenshot: image

P.S A gibberish date input has the same behavior of an invalid date input.

damithc commented 6 years ago

I think using a green status message for the invalid case is not suitable. Either make the whole thing orange or show two messages, one orange, one green? Is it possible to show what was the invalid date entered, for the user's reference? It's ok if that takes too much work.

tanhengyeow commented 6 years ago

@damithc Yup, doable. Here's the new behavior:

error 2

image

damithc commented 6 years ago

Looks good. May be phrase it as The following values were found to be invalid and the original values have been retained instead (or something like that)

wkurniawan07 commented 5 years ago

Resolved in V7.