Skip to content

[SQSERVICES-1911] Team features MLS E2E ID#3082

Merged
battermann merged 8 commits intodevelopfrom
SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id
Feb 28, 2023
Merged

[SQSERVICES-1911] Team features MLS E2E ID#3082
battermann merged 8 commits intodevelopfrom
SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Feb 13, 2023 •
edited
Loading

Checklist

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

@battermann battermann changed the title add feature config for E2E ID [SQSERVICES-1911] Team features E2E ID Feb 13, 2023
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 13, 2023
@battermann battermann force-pushed the SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id branch from e7e0ba1 to 81cf29e Compare February 14, 2023 08:34
@battermann battermann changed the title [SQSERVICES-1911] Team features E2E ID [SQSERVICES-1911] Team features MLS E2E ID Feb 14, 2023
@battermann battermann force-pushed the SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id branch from 81cf29e to 94d3928 Compare February 14, 2023 08:50
@battermann
Copy link
Contributor Author

battermann commented Feb 14, 2023

I like to clean up code along the way sometimes, I apologize in advance!

@battermann battermann force-pushed the SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id branch 2 times, most recently from 32db67d to 764efc3 Compare February 14, 2023 16:01
@battermann battermann marked this pull request as ready for review February 14, 2023 16:33
@battermann battermann requested a review from fisx February 14, 2023 16:33
supersven
supersven approved these changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@supersven supersven left a comment

Choose a reason for hiding this comment

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

Looks like a good and clean implementation 👍

Only a couple of nitpickings... 😉

libs/wire-api/src/Wire/API/SystemSettings.hs Outdated
schema =
object "SystemSettingsInternal" $
SystemSettingsInternal
<$> ssiSetEnableMls .= fieldWithDocModifier "setEnableMls" (description ?~ "whether MLS is enabled or not") schema
Copy link
Contributor

@supersven supersven Feb 14, 2023

Choose a reason for hiding this comment

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

From the department of nitpicking: Maybe, we should decide if we start Swagger descriptions in upper or lower case.

"whether MLS is enabled or not" vs. "Do not allow certain user creation flows"

(No, this is really not important. I just accidentally saw it.)

libs/wire-api/src/Wire/API/Routes/Public/Brig.hs Outdated
:> Get '[JSON] SystemSettingsPublic
)
:<|> Named
"get-system-settings-authorized"
Copy link
Contributor

@supersven supersven Feb 14, 2023

Choose a reason for hiding this comment

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

I would call this endpoint get-system-settings and the unauthorized one get-system-settings-unauthorized, because that reflects the path. However, this is likely a matter of taste.

libs/wire-api/test/golden/testObject_WithStatus_team_18.json
{
"config": {
"verificationTimeout": 1676377048
"verificationExpiration": 1676377048
Copy link
Contributor

@supersven supersven Feb 14, 2023

Choose a reason for hiding this comment

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

Where does this come from? 🤔

Copy link
Contributor Author

@battermann battermann Feb 15, 2023

Choose a reason for hiding this comment

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

This was related to earlier commits in the PR.

@battermann battermann force-pushed the SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id branch from cb42fc7 to 5425326 Compare February 27, 2023 14:26
@battermann battermann merged commit ce4ecea into develop Feb 28, 2023
@battermann battermann deleted the SQSERVICES-1911-team-features-set-enable-mls-set-enable-mlse-2-e-id branch February 28, 2023 16:01
This was referenced Mar 6, 2023
Merged
Merged
@battermann battermann mentioned this pull request Mar 15, 2023
Merged
lepsa pushed a commit to lepsa/wire-server that referenced this pull request Nov 28, 2023
@battermann @lepsa
[SQSERVICES-1911] Team features MLS E2E ID (wireapp#3082)
444094d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@supersven supersven supersven approved these changes

@fisx fisx Awaiting requested review from fisx

Assignees

No one assigned

Labels

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.

3 participants

@battermann @supersven @zebot

Comments