Skip to content

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 return nil 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