Skip to content

WPB-26474: extends code to meetings#5281

Open
blackheaven wants to merge 7 commits into
developfrom
gdifolco/WPB-26474-code-meetings
Open

WPB-26474: extends code to meetings#5281
blackheaven wants to merge 7 commits into
developfrom
gdifolco/WPB-26474-code-meetings

Conversation

@blackheaven

Copy link
Copy Markdown
Contributor

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

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 June 19, 2026 15:49
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jun 19, 2026
MakeKey ref -> case ref of
CodeReferentConv _ -> Code.mkKey ref
CodeReferentMeeting _ ->
error "CodeStore.Cassandra.MakeKey: meetings are not supported on cassandra"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would cause 500s, right? If so, we should handle this properly, so the error becomes a 4xx (maybe 404?) something to let the clients know that the server doesn't support this feature.

Comment thread libs/wire-subsystems/src/Wire/CodeStore/Postgres.hs Outdated
Comment thread libs/wire-subsystems/src/Wire/CodeStore/Postgres.hs Outdated
Comment on lines +101 to +104
mkKeyId :: (MonadIO m) => Id a -> m Key
mkKeyId ident = do
sha256 <- liftIO $ fromJust <$> getDigestByName "SHA256"
pure $ Key . unsafeRange . Ascii.encodeBase64Url . BS.take 15 $ digestBS sha256 (toByteString' cnv)
pure $ Key . unsafeRange . Ascii.encodeBase64Url . BS.take 15 $ digestBS sha256 (toByteString' ident)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we add something here to stop same meetingid and convid collide in the key id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was thinking of prefixing keys like conv:<SHA> but I'm afraid it would break everything, WDYT?

@blackheaven blackheaven requested a review from akshaymankar June 22, 2026 16: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.

3 participants