Guard user check on member destroy for invites
The following discussion from !152171 (merged) should be addressed:
-
@vij started a discussion: (+6 comments) non-blocking observation: if
user
isn't present I think this predicate will now returnnil
instead of a boolean, which might be unexpected for some callers of the method.looking at the tests for the method, it doesn't seem like we anticipate user being
nil
but then looking at the DestroyService, it seems like it's possible (invites?🤔 )I wonder if we should either return a boolean in this method, or remove this change and check for user presence in the destroy service (e.g.
member.user && member.is_using_seat
), WDYT?🤔
non-blocking as I don't feel strongly either way (except that changing behaviour of billing related methods feels more dangerous), and I suspect this probably doesn't have an impact as
nil
is considered falsey🤔
I guess we could consider adding test coverage to confirm this is expected to handle user not always being present in these tests?
e.g. something like:
context 'when not hosted on GL.com' do ... context 'when user is not present' do member.user = nil expect(member.is_using_seat).to be_falsey end