search

ekweirui / pe

0 stars 0 forks source link

Unique contact identifier should not be name since there are many people with the same name. #4

Open ekweirui opened 1 year ago

ekweirui commented 1 year ago

Since a hotel is able to host many guests, the chances of a name collision is pretty high. Thus I would expect a unique identifier like a passport number to be used as the unique person identifier instead for such an application.

nus-pe-script 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]

Bug: Wrong parameters used for Equality Check

Current Behaviour

  • Equality is being determined only by name.
  • Its Very very very common for multiple people to have the same name
  • At it's current implementation If two people named john smith cannot check into your hotel at the same time despite them being different people holding different phone numbers, and email addresses

Expected Behaviour

  • Equality should be determined by the combination of name, email and phone number, creating a more reliable "primary key".

Screenshots

Screenshot 2022-11-11 at 4.46.34 PM.png

[original: nus-cs2103-AY2223S1/pe-interim#4124] [original labels: type.FeatureFlaw severity.High]

Their Response to the 'Original' Bug

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

While we believe that this is a bug, considering that we are targeting small hotels, the chance of having 2 people with the same full name is very very very uncommon. Additionally, names can be differentiated using custom suffixes as a workaround. As such, we decided to prioritise implementing other features over making the duplicate check more extensible.

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]

Reason for disagreement: Via the module website,

image.png

The usage of a unique identifier like a passport number should be used as the unique person identifier instead. If done so, this issue would have been solved, and this is quite an easy-to-do fix as well since it's just an addition of one more field.

Therefore since the feature could have been implemented to work in a better way without much additional effort, I reject that this is a NotInScope.

:question: Issue severity

Team chose [severity.Low] Originally [severity.Medium]

Reason for disagreement: image.png

Via the module website, the severity should be low when it is unlikely to affect normal operations, only appears in very rare situations, and causes a minor inconvenience.

However, this is not the case as
1) Normal operations are severely affected as adding the guest into the app is the first and foremost step before the user can do anything with the app.
2) The situation does not appear only in rare situations. It appears frequently given that there are many users out there with similar names, not to mention a hotel can hosts hundreds or even thousands of these people at once.
3) This flaw does not pose a minor inconvenience only as not being able to add a user makes the app unusable.

Also for legality reasons and for scenarios such as police need check the guest list for criminals, using custom suffixes as a workaround should not be considered as a viable alternative.

Thus, it should be a medium severity, as it does indeed causes occasional inconvenience to the users.