vicelyav / pe

0 stars 0 forks source link

Duplicate id message does not match when tutor is added #10

Open vicelyav opened 1 week ago

vicelyav commented 1 week ago

Commands to reproduce: add 13132324 n/name3 p/123458997 e/johdd3doe@example.com a/123Fample Street c/Computer Science r/Student

add 13132324 n/name3 p/123458997 e/johdd3doe@example.com a/123Fample Street c/Computer Science r/Tutor

Expected: A person with this ID already exists in EduContacts

Actual: A person with this studentID already exists in EduContacts

It does not differentiate the roles of students and tutors.

nus-se-script commented 3 days ago

Team's Response

Under planned enhancements #4.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: I understand that you are planning on adding more unique identifiers to differentiate the roles of the contact, as shown in this #4 Planned enhancement:

Screenshot 2024-11-19 at 9.35.07 PM.png

However, this relates to unique identifier and not the exception message, the latter which was the focus of the bug that I reported. It is not mentioned in this segment of the Planned Enhancement that it will modify the exception message. In addition, due to the vagueness of this segment of Planned Enhancement, the exception message could be modified by having 'duplicate emails' or 'duplicate phone'. Meanwhile, in this bug, of particular issue is the use of 'studentID' which could mislead users that the duplicate contacts are all students, rather than having one of them being a Tutor.

Additionally, in my opinion, it is possible to modify the exception message to be less misleading or more specific based on the current functionality of studentID. For instance, in Planned Enhancement #2, it is proposed that the exception message for duplicate id is this:

Enhancement: Introduce specific error handling to detect and report duplicate IDs in user commands. This will involve: Validating commands to identify duplicate ID arguments. Displaying a clear error message, e.g., "Duplicate ID detected, only one allowed."

Here, ID seems to be used generally, which would not mislead the user from thinking that all contacts are students. Alternatively, the exception message could use the assigned roles to differentiate between student and tutor, thus avoiding the general use of studentID in the exception message.

As the Planned Enhancement did not specify that it will address this issue, and as it is possible that the exception message could be more specific with the functionality before the Planned Enhancements, I reported this issue as a bug.