JamesLiuZX / pe

0 stars 0 forks source link

Lack of input validation for date #3

Open JamesLiuZX opened 1 year ago

JamesLiuZX commented 1 year ago

The range seems to go from 0000 to 9999 in years. The range should be calculated from the maximum allowable age which is 199 as set by your team, and dates from the future should not be able to be added.

Screenshot 2023-04-14 at 2.34.21 PM.png

Screenshot 2023-04-14 at 2.35.17 PM.png

nus-pe-script commented 1 year ago

Team's Response

No details provided by team.

The 'Original' Bug

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

Reported Date Allowed to be in the Future

Steps to Produce

  1. Enter a reported date in the future, for example: edit 1 d/2023-01-02
  2. Submit the input.

Screenshot 2023-04-14 143817.png

Expected Behavior The input should be rejected with an appropriate error message, informing the user that the reported date cannot be in the future.

Actual Behavior The input is accepted without any error message.

Justification Allowing reported dates in the future can lead to incorrect data and confusion for users. The system should not allow dates in the future and provide clear error messages to guide the user in correct usage. This doesn't make sense as the field is named as Reported date.


[original: nus-cs2103-AY2223S2/pe-interim#4009] [original labels: type.FunctionalityBug severity.Medium]

Their Response to the 'Original' Bug

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

Hi!

Thanks for pointing that out! You're right that cases from the future should not be allowed to be entered into the app, and we could have shown an error message instead.

However, this validation was not part of our project scope at the time; we focused instead on more general date and date format validation.

Checking for dates in the future is something we might consider doing in a future version of the product, so we see this bug report as being not in scope.

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: The other bug focused on being able to input dates in the future, mine also highlights the fact that the regex accepts both the past and the present (0000 to 9999) instead of (1xxx- 2xxx), and the fact that it should be set to from current date minus the maximum allowable age which is 199 as set by your team, to the current date.


## :question: Issue response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** This is not considered `NotInScope`, because it is 1) not specified in UG as not supported or WIP, 2) it is accessible to the user. ![Screenshot 2023-04-20 at 7.10.03 PM.png](https://raw.githubusercontent.com/JamesLiuZX/pe/main/files/8ced18a9-f122-44c6-a0c1-bc14ecf536e8.png) Considering how important date time validation is in a time-sensitive epidemic tracker with the need to observe trends as time goes on, this is definitely a severe bug, exacerbated by the fact that there is no error message to notify users of typos/ incorrectly imported formats. You mentioned that "validation was not part of our project scope at the time; we focused instead on more general date and date format validation.", but you are contradicting yourself in this sentence. This is definitely part of the core validation that should be performed on dates, and is a `FunctionalityBug` as your team had 'worked on date and datetime validation' as mentioned, but the input validation did not work as expected, hence the input functionality is bugged due to it accepting erroneous inputs into the database while displaying a success message.