WPB-19716: Batch create Channels and Map them to User Groups #4790
Conversation
zebot
added
the
ok-to-test
blackheaven
force-pushed
the
gdifolco/WPB-19712_user-groups-update-endpoint
branch
2 times, most recently
from
db1b267 to
2dea4d2
Compare
blackheaven
force-pushed
the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
from
235d349 to
50eec40
Compare
blackheaven
changed the base branch from
develop
to
WPB-20728-background-job-for-user-group-to-channel-association
blackheaven
force-pushed
the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
from
f1d21c7 to
24e10b4
Compare
| - `--galley-url GALLEY_URL`: Galley service URL (e.g., `https://prod-nginz-https.wire.com`) | ||
| - `--brig-url BRIG_URL`: Brig service URL (e.g., `https://prod-nginz-https.wire.com`) |
There was a problem hiding this comment.
i think one url for the public api of the backend services is enough. especially given we'd love to merge brig, galley, spar into one huge "micro"-service :)
There was a problem hiding this comment.
Sure, but I did not find a way to have it locally, so I replicated from another tool.
Which one is it?
There was a problem hiding this comment.
they almost always the same, so just --wire-server-url? but i'm having second thoughts since you're saying this is copied from prior art. what would you do if you had to decide?
There was a problem hiding this comment.
I don't mind changing, I just have not find the correct one for local/docker environment.
If you can tell me which one is it, or where I can find it, I'll be happy to change, it was my first implementation anyway.
libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserGroupStore.hs
Outdated
Show resolved
Hide resolved
| createChannel manager services (Token token) userId teamId channelName = do | ||
| let url = services.galleyUrl.fromApiUrl <> "/v12/conversations" | ||
| body = | ||
| object |
There was a problem hiding this comment.
aren't there types for this in wire-api?
There was a problem hiding this comment.
I forgot this one, sorry
| Nothing -> | ||
| case parseMaybe (.: "id") obj of | ||
| Just convId -> pure $ Right convId | ||
| Nothing -> pure $ Left $ ErrorDetail 201 (object ["error" .= ("Failed to extract conversation ID" :: Text)]) |
There was a problem hiding this comment.
why "ErrorDetail 201"? is 201 the http status code? why not 4xx?
There was a problem hiding this comment.
We get the correct status (201), but not the correct body.
There was a problem hiding this comment.
i don't understand (and i've even had coffee today!). in this line, you construct something that looks like an http response object (with status, body, ...), probably for being logged or somoehow reported to the user. (right?)
the http response you're constructing is about an error condition, but the status code says otherwise. why would the status code and the body in the same response tell different stories?
There was a problem hiding this comment.
Initially it was to report unexpected status code, should I create a dedicated constructor for unexpected bodies?
|
nice! i'm done reading, i'll (probably :) approve once you have responded to my comments. |
blackheaven
force-pushed
the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
from
7a2fedc to
d1da213
Compare
blackheaven
changed the base branch from
WPB-20728-background-job-for-user-group-to-channel-association
to
develop
blackheaven
force-pushed
the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
4 times, most recently
from
4a437fb to
ef79da1
Compare
blackheaven
force-pushed
the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
from
ef79da1 to
99455bb
Compare
blackheaven
deleted the
gdifolco/WPB-19716_channel-batch-create-user-groups
branch
https://wearezeta.atlassian.net/browse/WPB-17126
Checklist
changelog.d