gavingoh99 / pe

0 stars 0 forks source link

No warning for email missing top-level domain #8

Open gavingoh99 opened 4 months ago

gavingoh99 commented 4 months ago

Currently, the regex used by SWEE allows theodore@example as a valid email address. However, such an entry is missing its top-level domain (usually .com) and thus introduces some ambiguity.

This could occur when the user receives partial information about the client or attempts to add the client details hastily, leaving out the top-level domain in the process.

Given that there are a few different top-level domains, this ambiguity could mean that the information retrieved from SWEE may be incomplete, leaving the user to attempt to find out the corresponding / matching domain on their own.

Steps to reproduce:

  1. add --name=Theodore Koo --phone=98001715 --email=theodore@example -- addr=Prince Street, Block 144, #19-14 --tags=Disabled --note=Lactose Intolerant

Resultant output:

Screenshot 2024-04-19 at 5.04.56 PM.png

To alleviate this, the team could potentially look to include a warning when the top-level domain has been detected as missing so that the user can make a choice on whether to update it or bear the responsibility of determining the top-level domain on their own!

nus-pe-script commented 4 months ago

Team's Response

According to https://en.wikipedia.org/wiki/Email_address#Domain:

TLDs are optional, example given, intranet email servers admin@example is valid.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Thanks for providing references to official sources.

I note that my report did not dispute the validity of emails missing their top-level domains. Rather, I foresee a possibility of ambiguity being introduced from missing a top-level domain. There could be a possibility of emails that are only differentiated by their top-level domains and with your team's insight, there is now an additional possibility of an email being for an intranet email server.

In the suggestion I provided, the ideal case would be to present a warning to the user rather than be overzealous and validate based off of the presence / absence of the top-level domain. This ensures that the user is making an informed choice when storing emails of such format - if the user intends to store an intranet email then they can simply ignore the warning, but on the other hand, if the storing of the email without a top-level domain is a result of leaving out the information or receiving incomplete information on the user's end, then they can decide for themselves whether they would want to go ahead with storing the email given the possible ambiguity that it could cause!

Without a warning, however, users are not informed of this and could be met with immense difficulty in retrieving the information when they need to, as a result of the ambiguity caused by the email.


## :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.VeryLow`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** It turns out that `severity.VeryLow` is reserved for cosmetic issues which I feel this is certainly not. During the practical exam, I foresaw the inconvenience caused by the ambiguity of emails lacking top level domains in determining what the precise email address is. Given that social workers which form a key part of the target user group of SWEE are likely to be interacting with different stakeholders such as clients stored in SWEE, they are likely to look to the application for the email address that had been stored. Without providing an appropriate warning when these clients were initially added to the application, users are likely to be met with inconvenience and immense difficulty as they have to take it upon themselves to decipher the actual email address, taking away from the convenience afforded by the application to begin with.