engyuhan / pe

0 stars 0 forks source link

Scheduling appointments on invalid dates rounds down the date. #12

Open engyuhan opened 5 days ago

engyuhan commented 5 days ago

Steps to Reproduce:

  1. Create an appointment for any client for any duration on 30th Feb 2025.

Expected: Error message: Invalid date

Actual: Appointment scheduled for Date: 28-02-25

Making a typo like this and not receiving any warning can lead to disastrous consequences.

Screen Shot 2024-11-15 at 5.37.41 PM.png

nus-pe-script commented 1 day ago

Team's Response

This is a duplicate issue

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Invalid february dates are treated as valid inputs

Steps to recreate:

  1. Add an appointment to a client with an invalid february date, e.g. apt 3 d/30-02-01 fr/24:00 to/24:00
  2. Observe that the appointment is added with the largest february date e.g. 28-02-01 (as shown in screenshot 1 below), instead of the input being treated as invalid.

This does not happen for other months, in which dates like 32-03-01 are treated as invalid (see screenshot 2) Screenshot 2024-11-15 at 5.34.38 PM.png Screenshot 2024-11-15 at 5.34.23 PM.png


[original: nus-cs2103-AY2425S1/pe-interim#2320] [original labels: severity.Low type.FunctionalityBug]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Thanks for raising this issue.

Our team has actually tested this and realised this bug was cased by the date parsing or validation logic, which is likely using a lenient mode for date handling when processing February dates, allowing them to "roll over" to the nearest valid date instead of throwing an error. For other months, the validation behaves more strictly.

We understand that this bug can cause issues for a small handful of users and hence given the low severity, which our team agrees with as well.

We will address this issue in the upcoming iterations by ensuring stricter date validation and adding comprehensive unit tests to prevent discrepancies.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your explanation]


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** As specified in the course website, a severity.Low bug should cause only a minor inconvenience. Not issuing an error message or even a warning could very likely lead the user to miss an appointment, which is a large inconvenience. ![Screen Shot 2024-11-19 at 1.46.17 PM.png](https://raw.githubusercontent.com/engyuhan/pe/main/files/a6947187-8ed7-44c4-a13c-90c9026f4569.png)