nus-cs2103-AY2223S2 / forum

12 stars 0 forks source link

EditCommand FeatureFlaw or Bug #333

Closed francisyzy closed 1 year ago

francisyzy commented 1 year ago

Hi Prof, when doing further testing on my work, I've encounter a bug and not sure if it would be classified as a featureFlaw.

When editing the 'patient', the patient must show up in the filtered list or else it would show that it cannot be found. It is not intended.

This is our UG before the feature freeze. It shows that the user can edit the patient using the ID and did not mention that the patient must show up in the filtered list.

I had to fix a similar bug for a different related command but did not catch this bug before PE-D.

Would like to check if I am able to fix this BUG or file it under planned enhancement

damithc commented 1 year ago

@francisyzy what is the exact intended behavior and the actual behavior? Also give a screenshot of the relevant UG segment. You can PM me using MSTeams if you are uncomfortable sharing those details here.

francisyzy commented 1 year ago

Intended behaviour: would be the patient can be edited when the patient list is filtered (patient does not show up in the current view).

Actual behaviour: will throw a CommandException if the patient is not currently in the filtered list. The patient index provided is invalid

Intended:

image

Actual:

image

Screenshot of the relevant UG section before feature freeze:

image
damithc commented 1 year ago

@francisyzy There is certainly something that needs fixing -- for one thing, UG says PATIENT_ID but the error message says patient index. Are they the same thing?

francisyzy commented 1 year ago

@damithc Thanks for catching the error message error. It should say PATIENT_ID instead of INDEX

damithc commented 1 year ago

@francisyzy If it is the patient ID, and the intended behavior is that even patients not in the currently visible list should be editable by specifying the patient ID, it is a bug that you can fix in v1.4

francisyzy commented 1 year ago

Thanks prof for the clarification!