Skip to content

WPB-24075: Add Wire Meetings delete invitation endpoint#5136

Merged
blackheaven merged 4 commits intodevelopfrom
gdifolco/WPB-24075-meetings-invitation-delete
Mar 19, 2026
Merged

WPB-24075: Add Wire Meetings delete invitation endpoint#5136
blackheaven merged 4 commits intodevelopfrom
gdifolco/WPB-24075-meetings-invitation-delete

Conversation

@blackheaven
Copy link
Contributor

https://wearezeta.atlassian.net/browse/WPB-24075

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@blackheaven blackheaven requested review from a team as code owners March 19, 2026 13:13
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Mar 19, 2026
Copy link
Contributor

@battermann battermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to address the set vs list concern.

Otherwise LGTM

Comment on lines +302 to +305
r1 <- postMeetings owner newMeeting
assertSuccess r1

meeting <- assertOne r1.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r1 <- postMeetings owner newMeeting
assertSuccess r1
meeting <- assertOne r1.json
meeting <- postMeetings owner newMeeting >>= getJSON 200

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed!

removeEmailStatement =
[resultlessStatement|
UPDATE meetings M
SET invited_emails = (SELECT array(SELECT unnest(M.invited_emails) EXCEPT SELECT unnest($1 :: text[]))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the code correctly, you treat the invitations as a set here, whereas in the PR for adding invitations, they are treated as a list. Per API it is possible to add duplicate invitations, but once an invitation is removed, the duplicates will be removed as well. I find this a bit inconsistent, and could cause confusion, or surprises. Can we maybe make this more consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have switched the other to set semantic!

@blackheaven blackheaven merged commit c0a20a1 into develop Mar 19, 2026
9 checks passed
@blackheaven blackheaven deleted the gdifolco/WPB-24075-meetings-invitation-delete branch March 19, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants