Skip to content

Pick the member branch by list contents so add_members is not treated as a removal - #59

Open
gpunto wants to merge 1 commit into
mainfrom
fix/pick-member-branch-by-list-contents
Open

Pick the member branch by list contents so add_members is not treated as a removal#59
gpunto wants to merge 1 commit into
mainfrom
fix/pick-member-branch-by-list-contents

Conversation

@gpunto

@gpunto gpunto commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

🔗 Issue Links

No tracking issue.

update_members picks the add/remove branch by whether remove_members is present, but an empty array
is truthy in Ruby:

remove_members = json['remove_members'] ? true : false
  • Clients generated from the OpenAPI models send every action list on every call, so an add_members
    request arrives carrying "remove_members":[]. The helper then takes the remove branch, ignores
    add_members, and broadcasts no member.added event.

  • The fix picks the branch by whether the list has entries, and defaults member_ids to an empty list so a
    body that omits both keys no longer raises NoMethodError on nil.each.

  • Behaviour per payload shape, old vs new:

    body before after
    add_members:["jc"], remove_members:[] REMOVE, [] ADD, ["jc"]
    add_members:[], remove_members:["jc"] REMOVE, ["jc"] unchanged
    add_members:[], remove_members:[] REMOVE, [] ADD, [] (no-op either way)
    add_members:["jc"] (no remove key) ADD, ["jc"] unchanged
    remove_members:["jc"] (no add key) REMOVE, ["jc"] unchanged
    {} ADD, nil then raises ADD, []

    Only the first and last rows change, so bodies from the current clients keep their existing behaviour.

🧪 Testing Notes

Verified the branch selection for each shape above against both the old and the new expression. No E2E test
adds a member from the app today, which is why this has been invisible; the fix is what makes such a test
possible.

Please verify the E2E test runs:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant