Skip to content

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Sep 19, 2022

https://wearezeta.atlassian.net/browse/SQSERVICES-1643

Checklist

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

@battermann battermann temporarily deployed to cachix September 19, 2022 08:10 Inactive
@battermann battermann temporarily deployed to cachix September 19, 2022 08:10 Inactive
@battermann battermann force-pushed the SQSERVICES-1643-backend-servantify-brig-account-api-3-post-activate branch from 388a9bd to 0425e04 Compare September 19, 2022 08:10
@battermann battermann temporarily deployed to cachix September 19, 2022 08:10 Inactive
@battermann battermann temporarily deployed to cachix September 19, 2022 08:10 Inactive
@battermann battermann changed the base branch from develop to SQSERVICES-1643-backend-servantify-brig-account-api-1-get-activate September 19, 2022 08:12
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 19, 2022
@battermann battermann requested a review from fisx September 19, 2022 08:21
@battermann battermann marked this pull request as ready for review September 19, 2022 08:21
Comment on lines +107 to +112
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
maybeActivationKeyTargetFromTuple :: (Maybe ActivationKey, Maybe Phone, Maybe Email) -> Parser ActivationTarget
maybeActivationKeyTargetFromTuple = \case
(Just key, _, _) -> pure $ ActivateKey key
(_, _, Just email) -> pure $ ActivateEmail email
(_, Just phone, _) -> pure $ ActivatePhone phone
_ -> fail "key, email or phone must be present"
maybeActivationKeyTargetFromTuple :: (Maybe ActivationKey, Maybe Phone, Maybe Email) -> Parser ActivationTarget
maybeActivationKeyTargetFromTuple = \case
(Just key, _, _) -> pure $ ActivateKey key
(_, _, Just email) -> pure $ ActivateEmail email
(_, Just phone, _) -> pure $ ActivatePhone phone
_ -> fail "key, email or phone must be present"

I like symmetry! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

does was a lie before, right? the json instances didn't know about this field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I noticed that, too.

@battermann battermann force-pushed the SQSERVICES-1643-backend-servantify-brig-account-api-1-get-activate branch 2 times, most recently from 776df23 to ea4975a Compare September 20, 2022 08:28
@battermann battermann force-pushed the SQSERVICES-1643-backend-servantify-brig-account-api-3-post-activate branch from 0425e04 to 0d6e3e7 Compare September 20, 2022 08:29
@battermann battermann temporarily deployed to cachix September 20, 2022 08:29 Inactive
@battermann battermann temporarily deployed to cachix September 20, 2022 08:29 Inactive
@battermann battermann temporarily deployed to cachix September 20, 2022 08:39 Inactive
@battermann battermann temporarily deployed to cachix September 20, 2022 08:39 Inactive
@battermann battermann force-pushed the SQSERVICES-1643-backend-servantify-brig-account-api-1-get-activate branch from ca38058 to 665bb3c Compare September 20, 2022 13:19
Base automatically changed from SQSERVICES-1643-backend-servantify-brig-account-api-1-get-activate to develop September 20, 2022 14:34
@battermann battermann force-pushed the SQSERVICES-1643-backend-servantify-brig-account-api-3-post-activate branch from 700de27 to c4d0f6f Compare September 20, 2022 14:35
@battermann battermann temporarily deployed to cachix September 20, 2022 14:35 Inactive
@battermann battermann temporarily deployed to cachix September 20, 2022 14:35 Inactive
@battermann battermann merged commit 7dd6ed9 into develop Sep 21, 2022
@battermann battermann deleted the SQSERVICES-1643-backend-servantify-brig-account-api-3-post-activate branch September 21, 2022 07:57
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.

4 participants