Update editCommand and deleteCommand logic #131
Conversation
so that it preserves UUID of editedPerson
Codecov ReportAttention: Patch coverage is
|
shardhrv
requested review from
chuajunyu and
jan-kai1
and removed request for
chuajunyu
| if (volunteers.contains(personToEdit)) { | ||
| volunteers.remove(personToEdit); | ||
| volunteers.add(editedPerson); | ||
| } |
There was a problem hiding this comment.
Can't think of anyway better to do this rn but this does seem abit funny caz we have to check every set and add / remove, maybe we can think of something better next time
There was a problem hiding this comment.
Perhaps we could abstract out this removal and addition into a method called editPersonInRoleSet to make it more readable and less repetitive, but otherwise the implementation seems sound
| * Removes person from all roles if they are present in that role | ||
| * @param personToRemove Person that will be removed | ||
| */ | ||
| public void removePerson(Person personToRemove) { |
There was a problem hiding this comment.
Looks good, so deleting a person will remove them entirely
| if (volunteers.contains(personToEdit)) { | ||
| volunteers.remove(personToEdit); | ||
| volunteers.add(editedPerson); | ||
| } |
There was a problem hiding this comment.
Perhaps we could abstract out this removal and addition into a method called editPersonInRoleSet to make it more readable and less repetitive, but otherwise the implementation seems sound
Updated the logic for these commands execution, as it no longer makes sense to store UUID in EventStorage over the Person themselves. This is because the latter would require a major rework of the current implementation of the interactions between ModelManager and EventManager.