Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WPB-10658 invitation and acceptance of individual users to teams #4229

Merged
merged 7 commits into from
Sep 17, 2024
Merged

WPB-10658 invitation and acceptance of individual users to teams #4229

merged 7 commits into from
Sep 17, 2024

Conversation

battermann
Copy link
Contributor

@battermann battermann commented Sep 5, 2024 •
edited
Loading

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

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines
  • Use the same email templates as for normal team invitations. These will be replaced in a subsequent PR.
  • Update all relevant environments before merging (staging, prod?)
  • Add password protection on accepting the invitation

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 5, 2024
@battermann battermann changed the title Wpb 10658 invitation and acceptance of individual users to teams WPB-10658 invitation and acceptance of individual users to teams Sep 9, 2024
mdimjasevic
mdimjasevic approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@mdimjasevic mdimjasevic left a comment

Choose a reason for hiding this comment

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

There are still a few small things to do, but this looks good.

fisx
fisx reviewed Sep 11, 2024
View reviewed changes
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

i can't think of anything that we may have missed, but i'm not entirely confident that we haven't... 🤔

lots of comments, most of them do not require action.

charts/brig/templates/configmap.yaml Show resolved Hide resolved
charts/brig/templates/configmap.yaml Show resolved Hide resolved
integration/test/Test/Teams.hs Show resolved Hide resolved
integration/test/Test/Teams.hs Outdated Show resolved Hide resolved
libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/UserEvent.hs Show resolved Hide resolved
services/brig/src/Brig/Team/API.hs Outdated Show resolved Hide resolved
services/brig/src/Brig/Team/API.hs Outdated Show resolved Hide resolved
integration/test/Test/Teams.hs Show resolved Hide resolved
services/brig/src/Brig/Team/API.hs Outdated Show resolved Hide resolved
services/brig/src/Brig/Team/Email.hs Show resolved Hide resolved
fisx
fisx requested changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

https://wearezeta.atlassian.net/browse/WPB-10658?focusedCommentId=128473

@battermann battermann marked this pull request as ready for review September 11, 2024 11:30
@fisx
Copy link
Contributor

fisx commented Sep 12, 2024 •
edited
Loading

hints for merge conflict resolution with #4218:

  • Brig.Team.DB had been moved to Wire.InvitationCodeStore by them. git diff $(git merge-base origin/WPB-10658-invitation-and-acceptance-of-individual-users-to-teams)..origin/WPB-10658-invitation-and-acceptance-of-individual-users-to-teams -- services/brig/src/Brig/Team/DB.hs shows the changes we need to make happen there.
  • findTeamInvitation (local function) in Brig.API.User has changed by them and has been moved to the top-level by us.
  • createInvitation' in Brig.Team.API has been changed by both them and us.

All other conflicts should be trivial.

us:

git diff $(git merge-base origin/develop origin/WPB-10658-invitation-and-acceptance-of-individual-users-to-teams)..origin/WPB-10658-invitation-and-acceptance-of-individual-users-to-teams

them:

git diff $(git merge-base origin/develop origin/mangoiv/wpb-8887)..origin/mangoiv/wpb-8887

@fisx
WPB-10658: invite personal users into teams.
440606c 
failing test

basic implementation with holes

email sending function no impl

dummy endpoint for accepting invitation

failing test that user is added to team

todo honor timeout

refactoring to make findInvitation reuseable

accept invitation

extend test, publich events

test for team id, include team in es doc version

assert notification to user sent

when fanoutsize is low

Add email invitation placeholder

Revert "when fanoutsize is low"

This reverts commit 0834e48.

update nginz confs

added errors

clean up/renaming

email sending

Add change logs

setting invite url in configmap

verify invitation url query parameter

release notes

clean up

move code to http helpers

extend golden tests

renaming

use same timeout settings for all invitations

fix

format email html

add password test (failing)

Check if the user password is correct

fix test in CI

test user cannot join multiple teams

Simplify a helper function type

Guard against accepting multiple team invitations

Throw a suitable invitation not found error
@fisx
Copy link
Contributor

fisx commented Sep 13, 2024

backup branch before rebase: WPB-10658-invitation-and-acceptance-of-individual-users-to-teams-bakleif

@fisx fisx force-pushed the WPB-10658-invitation-and-acceptance-of-individual-users-to-teams branch from 70d8dd5 to 440606c Compare September 13, 2024 15:41
@fisx
Copy link
Contributor

fisx commented Sep 13, 2024

there are some conflicts left from the rebase, just follow the compiler errors.

@battermann battermann force-pushed the WPB-10658-invitation-and-acceptance-of-individual-users-to-teams branch from b527e6c to 78a4068 Compare September 16, 2024 10:51
fisx
fisx approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

i'd like to look into acceptTeamInvitationByPersonalUser, but if you don't want to do that here that's fine, too. :)

services/brig/src/Brig/App.hs Outdated Show resolved Hide resolved
services/brig/src/Brig/Team/API.hs
Local UserId ->
AcceptTeamInvitation ->
(Handler r) ()
acceptTeamInvitationByPersonalUser luid req = do
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this function could be reorganized to be a little clearer?

@fisx fisx merged commit 64f156a into develop Sep 17, 2024
10 checks passed
@fisx fisx deleted the WPB-10658-invitation-and-acceptance-of-individual-users-to-teams branch September 17, 2024 06:26
@zebot zebot mentioned this pull request Oct 9, 2024
Closed
This was referenced Oct 23, 2024
Closed
Closed
@zebot zebot mentioned this pull request Oct 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fisx fisx fisx approved these changes

@mdimjasevic mdimjasevic mdimjasevic approved these changes

Assignees
No one assigned
Labels
echoes/initiative: personal-users-to-team-users... ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@battermann @fisx @mdimjasevic @zebot