Team's Response

This issue is the exact same issue as the one brought up in the referenced bug (#3541). Both these bugs cannot be fixed independently.

The 'Original' Bug

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

Editing email

Description: Error editing email for existing contact. Error message says student already exists but there is only 1 student in the entire contacts list. Emails tried: "validemail@gmail.com", "validemail@example.com", "validemail@yahoo.com". However, when editing in the exact same email the contact already has, it results in a successful edit.

Steps to reproduce:

delete all applications until left with 1

edit 1 e/validemail@gmail.com

error message as in image

Expected output: successful execution of editing email

Actual output: error message

Screenshots:

_{[original: nus-cs2103-AY2122S2/pe-interim#3513] [original labels: type.FeatureFlaw severity.High]}

Their Response to the 'Original' Bug

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

Hi! This issue is extremely similar to bug #2678 and has been caused due to a small oversight in the same place in our code base.

Throughout our project, we have considered student numbers and email ids to be the two unique identifiers of students and thus both the checks go hand in hand in all places together.
       if ((!personToEdit.getEmail().equals(editedPerson.getEmail())
               || !personToEdit.getStudentNumber().equals(editedPerson.getStudentNumber()))
               && model.hasPerson(editedPerson)) {
           throw new CommandException(MESSAGE_DUPLICATE_PERSON);
       }
The issue arose here and the hasPerson boolean check shouldn't have been included in conjunction with this if statement. hasPerson also lacks the unique identifier check (both student number + email) in a certain place that we, unfortunately, missed.

Our reasoning for moving this issue to a low severity is the same for bug #2678 as attached below:

Severity Low is defined as "A flaw that is unlikely to affect normal operations of the product. Appears only in very rare situations and causes a minor inconvenience only." and we believe this is what is applicable here as well.

As the developing team, the import-csv command was implemented to optimise getting tasks done faster for the user. Our recommended mode for creating your student list is by using import-csv and this method is undoubtedly less prone to (data-entry) errors if the file is exported from LumiNUS. We don't expect there to be much, if at all any, need for editing a student number since the information provided by LumiNUS will be accurate.

Going down the same path, manual addition of students is projected to be in rarer situations and if the user made an error in the process then yes, as pointed out, our application has a bug.

Nonetheless, if we look at TAilor on a whole, there aren't many commands that are dependent on the student number attribute of a Student.

...

The only situation where you could need to use the matriculation number is when you want to find one particular student. This process may be hindered due to the wrong student number being entered in the first place (and the edit option being buggy), however, we have a functional alternative which is to find using the unique email address.

...

Looking at the chances of this situation occurring, inconveniences being caused (while using other commands), and functional alternatives, we are happy to accept this bug at low severity.

Final thoughts:

The chances that a user, who is extremely comfortable typing and using CLI, creates an error in BOTH the Student Number and the email address is very very rare. If they mess up one, then finding a particular student using the other unique identifier is always a working alternative.

We acknowledge the mistake and are sad that it wasn't rectified before but simply believe that it will be a low severity when we actually look into the possibility of the situations occurring, realistically.

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)

[ ] I disagree

Reason for disagreement: [replace this with your explanation]

:question: Issue type

Team chose [type.FeatureFlaw] Originally [type.FunctionalityBug]

[ ] I disagree

Reason for disagreement: [replace this with your explanation]

:question: Issue severity

Team chose [severity.Low] Originally [severity.Medium]

[x] I disagree

Reason for disagreement: I understand your point. However, I disagree that this is a of severity.Low. In particular, there is not just one issue. In particular,

I agree that this is a design problem, which must be fixed.
In addition, notice that the error message is very wrong (it does not help to address the problem at all). In this case, it is said that student already exists, but it is not the case as we are trying to edit the email of the student.
student email is not fixed. In particular, even for NUS email, students' email can be changed from their nusnet id to more readable form. For example, e1234567@u.nus.edu to myname@u.nus.edu. This is quite common especially for Y1 students (who have not changed their email addresses).
As stated in their explanation, I agree that changing student number especially when it is imported from LumiNus csv is unlikely. But from number 3 you can see why the students' email in LumiNUS csv might not be fixed across the semester.

With that, I feel that this is not as "trivial" as what the team's response seems to address.

As you can see from the website, it will cause some inconvenience to the users while being able to continue using the application. Therefore, I still think that my original severity is more accurate.

albertsutz / pe

Cannot edit email for existing contact #7