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.

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

WPB-6258 Connection request from deleted user #3861

Conversation

battermann
Copy link
Contributor

@battermann battermann commented Jan 31, 2024

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

Checklist

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

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 31, 2024
@battermann battermann marked this pull request as ready for review February 6, 2024 09:08
@battermann battermann marked this pull request as draft February 7, 2024 16:10
@battermann battermann marked this pull request as ready for review February 8, 2024 08:44
@battermann battermann requested a review from stefanwire February 8, 2024 10:21
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.

please ignore my comments, it's all good to go! :)

@@ -401,3 +402,14 @@ testFederationAllowMixedConnectWithRemote =
connectTwoUsers alice bob
where
defSearchPolicy = "full_search"

testPendingConnectionUserDeleted :: HasCallStack => Domain -> App ()
testPendingConnectionUserDeleted domain = do
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
testPendingConnectionUserDeleted domain = do
testPendingConnectionUserDeleted bobsDomain = do

? I sometimes feel we're trying really hard to make identifiers mean as little as possible in /integration. could be just me, though :)

now
(ucConvId uc)
let e = ConnectionUpdated ucCancelled Nothing Nothing
onConnectionEvent deleted conn e
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason the local and the remote part look so different? (just curious)

sendCancelledEvent :: Local UserId -> Remote UserConnection -> m ()
sendCancelledEvent luidDeleted ruc = do
runExceptT (sendConnectionAction luidDeleted Nothing (qUnqualified . ucTo <$> ruc) RemoteRescind) >>= \case
-- I don't think we want to abort the operation since this is running asynchronously and cannot be retried anyway
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't it be retried? i'd expect this to go back to the rabbit queue and then get picked up again in a bit?

@battermann battermann requested a review from fisx February 14, 2024 11:19
@battermann battermann merged commit b7abf88 into develop Feb 14, 2024
8 checks passed
@battermann battermann deleted the WPB-6258-be-connection-request-from-deleted-user-cannot-be-rejected branch February 14, 2024 14:24
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