-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propagate delete companion #278
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
||
// If a dependent user, remove self as the primary traveler in trips created by other users. | ||
// TODO: Should we alert the user who created the trip of the deletion? | ||
Persistence.monitoredTrips |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is probably a case here for just deleting the trip if the primary traveler is no longer travelling. Perhaps that would start to complicate things though?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good observation. Could preventing tracking if a companion is set but no primary traveler is defined be a better way to handle that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think deleting the trip makes sense if there is no primary traveler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created issue #282 to track that.
|
||
/** | ||
* Remove the specified user as observer from the specified trip. | ||
* @return true if one or more related users gor invalidated, false otherwise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. are
not gor
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 528d8bb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just one tiny nit
.getFiltered(Filters.eq("companion.email", email)) | ||
.forEach(trip -> removeCompanion(this, trip)); | ||
|
||
// If a companion user, invalidate relationship in trips where they are companions and observers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. this comment is a duplicate of the comment above.
|
||
// If a dependent user, remove self as the primary traveler in trips created by other users. | ||
// TODO: Should we alert the user who created the trip of the deletion? | ||
Persistence.monitoredTrips |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think deleting the trip makes sense if there is no primary traveler.
.forEach(trip -> removeCompanion(this, trip)); | ||
|
||
// If a companion user, invalidate relationship in trips where they are companions and observers. | ||
// TODO: Should we alert the user who created the trip of the deletion? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would hurt to notify the creator of the trip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checklist
dev
before they can be merged tomaster
)Description
This PR addresses #271 (comment).
When a user deletes their account, they are removed as primary traveler on trips saved by others,
and they are invalidated as companions and observers on trips saved by others.