search

Chronoxy / pe

0 stars 0 forks source link

[Add/Edit] Email Field #6

Open Chronoxy opened 4 days ago

Chronoxy commented 4 days ago

Background

Enter an email address based on the specifications mentioned, with all alphanumerical characters and these special characters +._- for local-part. similarly for domain's restriction

image.png

Test Case

local-part

add n/John Lee nric/S9369777D p/91234567 e/special+._-char@example.com a/123 Medical D role/PATIENT t/Chicken

domain

add n/John Lee nric/S9369777D p/91234567 e/domain@exam.p-a a/123 Medical D role/PATIENT t/Chicken

Expected Behaviour

Contact created succesfully

Encountered Behaviour

local-part

image.png

domain

image.png

Suggestion

Correct your error message

nus-pe-bot commented 16 hours ago

Team's Response

No details provided by team.

The 'Original' Bug

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

"a__a" is not accepted as the local part of the email

Description:

According to the user guide, the local part of the email is valid as long as it follows the following rules:

Local-part: Can contain alphanumeric characters and these special characters: + _ . -

  • Cannot start or end with special characters.

Based on the validation rules above, a__a should be a valid local part. However, when attempting to enter any email with a__a as the local part, the application shows an error stating that the email format is incorrect.

Steps to reproduce:

Enter the following command:

add n/John Doe nric/S6483749D p/98765432 e/a__a@example.com a/311,
Clementi Ave 2, #02-25 t/backPain t/diabetic role/patient

An error stating that the email has an invalid format will be displayed.

[original: nus-cs2103-AY2425S1/pe-interim#902] [original labels: severity.Medium type.FunctionalityBug]

Their Response to the 'Original' Bug

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

I acknowledge this is a valid functionality bug as our implementation doesn't match what's documented in the UG regarding email validation rules. However, I would like to suggest downgrading the severity from Medium to Low because:

  • This only affects a very specific edge case - emails containing exactly two consecutive underscores
  • Single underscores work as expected (e.g., 'a_a@example.com' is accepted)
  • This appears only in rare situations as double underscores in email addresses are uncommon in regular usage
  • There's a simple workaround of using single underscores instead
  • The limitation doesn't prevent users from performing any core functionality of the application
  • Most users would still be able to use their regular email addresses without issue

While we agree this is a bug that should be fixed to fully match our documented rules, its impact on regular usage is minimal enough to warrant Low severity classification.

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: While it is similar to some extent, with the 'Original' bug report mentioning a single test case of a__a the use of double \\s, my bug report submitted was far more than just a single edge case as responded by the developer team to the 'Original' bug.

## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** Thank you for your response. As mentioned in my objection of my bug report being a duplicate, the bug report i submitted offers more than what the "original" bug report mentioned. Breaking down the `Email` field into 2, namely local-part and domain, as advised in your User Guide and error message. they each present some form of error which is not aligned with what was advised by your error message. Some example test cases for local-part which would still cause an error include but is not limited to `a..a`, `a...a`, `a++a`, `a--a`, `a-_.a`, and many more. With the shown error message, users would not know what is wrong with their input field for local-part. As for domain, these test cases will also cause an error. This includes but not limited to `exa.p-a`, `exa.p-a.da`, and many more. Also with the shown error message, users would not know what is wrong with their input field for domain. As explained above, because of the incorrect error message and User Guide, as a user i will struggle with trying to fix the error without knowing where the error is coming from, making me frustrated and might just not use the application completely. As this does not "affect a very specific edge case - emails containing exactly two consecutive underscores", there are many more cases that this error will appear. I agree with that it may be uncommon, but it is not impossible. With the user not knowing where the error is coming from, this will cause a significant amount of issues and stress. As such, I firmly state my stance for the severity to remain at `Medium`. Thank you.