hovanleong / pe

0 stars 0 forks source link

Duplicate telegram handles and github usernames are allowed #1

Open hovanleong opened 4 days ago

hovanleong commented 4 days ago

Using the app, I called two commands, add name/John Doe email/johnd@example.com telegram/@john github/swag-john33 and add name/Jon Doe email/johnd@example.com telegram/@john github/swag-john33.

This was allowed by the application even though emails, telegram handles and github usernames are unique and cannot be shared by two different individuals. In the case of target users, CS2030S tutors may easily copy paste their students telegram handles/github usernames incorrectly, especially if their students have similar names such as John Doe and Jon Doe.

Here is the screenshot attached: Screenshot 2024-11-15 at 4.29.10 PM.png

The Q&A component slightly touches on this and states "KonTActs recognizes that students may use the same usernames and emails across different points of contact. To provide greater flexibility in the application, KonTActs permits duplicate entries for email addresses, Telegram handles, and GitHub usernames." but I do not feel it is a valid reason as all NUS students are assigned with a unique NUSNet email upon matriculation.

I would suggest that duplicate entries for tele handles and github users should be checked and handled instead of just being accepted, considering that most CS2030S tutors are undergraduates whom work very hard and might be very tired and easily input inaccurate info when entering students' data into the database.

nus-pe-bot commented 1 day ago

Team's Response

It is a subset of #798. Both is about allowing duplicate entries for telegram & github.

The 'Original' Bug

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

Add Command allows duplicate telegram/github/email handles

Steps to Reproduce

  1. add name/Test1 email/test@email.com telegram/@telegram github/test
  2. add name/Test2 email/test@email.com telegram/@telegram github/test

Expected Outcome

When an inputted student has duplicate emails, telegram handles, or github account, they probably mean the same person. Even if they have different points of contact it makes the list harder to navigate when there are accidental duplicates referring to the same person.

Actual Outcome

No validation for this duplication is performed and contacts are added without raising warnings.


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

Their Response to the 'Original' Bug

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

Currently it is not implemented to prevent overzealous input validation and we also acknowledge that some students might use the same email / telegram maybe for group projects. In the future implementations, we would ensure that duplicate telegram IDs, github accounts and emails will be checked.

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 response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** Sure, students may want to have 1 POC pertaining to group projects. However, let's remember that in a university setting, students are still individual students receiving individual grades at the end of the sem. There will always be an individualistic element to NUS CS academia, which renders each student's well-being to be checked upon. Not to mention, there are zero group components/elements in CS2030S (the dev team's UG specifically mentions that this app is for CS2030S tutors only). Lastly, I explicitly mentioned in my initial bug report that "but I do not feel it is a valid reason as all NUS students are assigned with a unique NUSNet email upon matriculation". They did not address this point. Given that this is an addressbook for CS2030S tutors, it would not be ideal if tutors cannot make sure that they have every student's contact details at their fingertips. I humbly feel that the reason provided by the dev team is unconvincing. I am okay with being generous and accepting the dev team's lowering of the severity from `severity.Medium` to `severity.Low`. But as per the explanations above, for me it is a sure `type.FeatureFlaw` and not `response.NotInScope`.
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]