ARPspoofing / pe

0 stars 0 forks source link

Invalid NUS email allowed on edit #5

Open ARPspoofing opened 1 year ago

ARPspoofing commented 1 year ago

An invalid NUS email should be allowed. An NUS email should be e, followed by 7 digits, and end with @u.nus.edu. However, editing it allows the local name to have less than 7 digits.

Steps to reproduce:

  1. add n/Wen Li e/e07123456@u.nus.edu p/91234567 a/Kent Ridge PGPR tele/@wenlisan r/Very hardworking :)
  2. edit 1 e/e1@u.nus.edu

Screenshot 2023-04-14 at 2.28.55 PM.png

Solution: Perhaps we could add a simple regex to ensure the length of the email is met, and contains 7 digits. I think that will be a good fix for this feature.

nus-se-bot 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]

Invalid email domain name allowed on edit

I do not think we should allow the user to edit the domain name to simply at least 2 characters. There should at least be a check to ensure it ends with .com or .edu etc.

Steps to reproduce:

  1. clear
  2. add n/Wen Li e/e07123456@u.nus.edu p/91234567 a/Kent Ridge PGPR tele/@wenlisan r/Very hardworking :)
  3. edit 1 e/e@aa

This solution should be solved if the domain name is strictly @u.nus.edu. This way, we can better scope it to the product since it is meant for NUS TAs, which means the students must be NUS students. Since the students are NUS students, it also means they will have a valid NUS email.


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

Their Response to the 'Original' Bug

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

While we do understand your point about the invalid email address, we however did not intend for the email field to strictly be the student's NUS email, and we intended for it to include a student's external email address as well. In reality, an email's domain name can be changed easily, so there are many permutations of valid email addresses. We also feel that this does not really affect the user experience, except for cases where the user make typos to the email domain, where it can be easily edited.

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.Rejected`] - [x] I disagree **Reason for disagreement:** It does affect the user's experience. When an invalid email targeted specifically towards NUS students is entered, it should be promoted an invalid. Should you have not implemented the checking of valid nus emails, it should have been mentioned under planned enhancements, rather than rejecting this. It is important to check for validity of an email, just like what you guys did for telegram handle. From your argument, there should not need to have validity check for telegram handle, which clearly you guys implemented it. Hence, I do not see why this bug is rejected.
## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** This is a somewhat important feature for students, as it is one of the only few ways to ensure they are contacted, if they do not have any telegram. Therefore, at least some form of validity check for the top level domain (.com, .edu) etc should have been implemented. Since it is one of the only few added features of the application, it should remain at medium severity.