Possible to bypass conflict checks after some editing
Opened this issue · 1 comments
In the user guide, it is stated that "You cannot add duplicate buyers that have the same phone number or email.".
The spirit of this conflict checks can be bypassed, as shown:
- Start with default data
editbuyer 3 -n John Doe -e alexyeoh@example.com -r 40000-50000 -pr HIGH
- Now, both index 1 and 3 have the same email address.
I noticed that app checks to prevent names, numbers and emails duplicates, although this example is only for emails. I believe it also extendable to names and numbers.
Team's Response
No details provided by team.
The 'Original' Bug
[The team marked this bug as a duplicate of the following bug]
Editting a buyer detail does not check for duplicate phone number
A buyer can be editted to have the same phone number as another buyer.
If user will to exit the app, subsequent opening of the app will not be able to read the data due to duplicate values, causing all buyer data to be lost.
[original: nus-cs2103-AY2223S1/pe-interim#4848] [original labels: type.FunctionalityBug severity.High]
Their Response to the 'Original' Bug
[This is the team's response to the above 'original' bug]
Unfortunately, the team did not manage to catch this bug when we were testing.
Original Implementation
The problem arises in this statement.
We throw an exception if:
- The edited buyer is not the same as the buyer to be edited in the list, AND
- We can still find the edited buyer in the buyer list.
This is only valid based on the assumption that:
- If the edited buyer is the same as the buyer to be edited,
- then the edited buyer will not be the same as other buyers, and thus no duplicates are formed.
Problems due to changes made to
isSameBuyer
However, we changed the implementation of
isSamebuyer
. Now,isSameBuyer
returns true if either phone number OR email is the same.Hence, the previous assumption made is false. It is now possible for an edited buyer to be considered the same as more than 1 buyer (consider a buyer with the same phone number as 1 buyer and the same email as another buyer).
Thus, there could be a case where the edited buyer IS the same as the buyer to be edited in the list (hence failing condition 1 and not throwing the exception), but is also the same as another buyer in the list, thereby introducing a duplicate buyer.
- Edit either a buyer's email or phone number only such that they are the same as a different buyer in the buyer list
!buyerToEdit.isSameBuyer(editedBuyer)
: returnsfalse
, as there is still an unedited field (either email or phone number) that is is the same as the original buyerSolution
The main idea of duplicate checking is that we should only be checking whether our newly edited buyer is the same as all the other buyers in the list. It does not matter if it is the same as the buyer to be edited.
What we could have done is modify the
hasBuyer
method to include an extra argument that takes in an index to be excluded from consideration. In this way, we can exclude the buyer to be edited from being checked if it is the same as the edited buyer.Hence, this change would basically solve all problems with buyer duplicates (both phone numbers and emails).
Items for the Tester to Verify
❓ Issue duplicate status
Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)
- I disagree
Reason for disagreement: [replace this with your explanation]
❓ Issue severity
Team chose [severity.High
]
Originally [severity.Low
]
- I disagree
Reason for disagreement: [replace this with your explanation]