Skip to content

Conversation

@fisx
Copy link
Contributor

@fisx fisx commented Jan 21, 2026

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

Checklist

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

@fisx fisx changed the title [WPB-22549] connect users to apps. WPB-22549: connect users to apps. Jan 21, 2026
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 21, 2026
@fisx fisx changed the title WPB-22549: connect users to apps. [WPB-22549] connect users to apps. Jan 21, 2026
@fisx fisx added not-ok-to-test Not approved for running tests in CI, this label is ignored if ok-to-test also exists on a PR and removed ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist labels Jan 21, 2026
@fisx fisx force-pushed the WPB-22549-connect-user-to-app branch 6 times, most recently from 14b7ab8 to 9d436c0 Compare January 28, 2026 15:26
@battermann battermann force-pushed the WPB-22549-connect-user-to-app branch from e1925d0 to 89c8d44 Compare February 3, 2026 17:11
@battermann battermann removed the not-ok-to-test Not approved for running tests in CI, this label is ignored if ok-to-test also exists on a PR label Feb 4, 2026
Nothing -> case userTeam u of
Nothing -> pure Nothing
Just tid -> do
mApp <- AppStore.getApp (userId u) tid
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't this happen in UserStore?

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 5, 2026
Store.createApp app
Store.createUser u Nothing
now <- toUTCTimeMillis <$> get
void $ addTeamMember u.id tid (Just (tUnqualified lusr, now)) R.RoleMember
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an essential fix. Before this, apps had a user record, but no team member record. (Excercise to the reader: why was that bad?)

@fisx fisx force-pushed the WPB-22549-connect-user-to-app branch from 4f892ce to a06642e Compare February 10, 2026 15:33

-- the same goes for apps from the same team
postConnection mem1 app `bindResponse` \resp -> do
resp.status `shouldMatchInt` 403
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this still fails, because the response status is 404. shouldn't be too hard to fix.

@fisx fisx force-pushed the WPB-22549-connect-user-to-app branch 2 times, most recently from bfbd6fa to 1186a9c Compare February 11, 2026 07:21
Copy link
Contributor

@battermann battermann left a comment

Choose a reason for hiding this comment

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

I think the new column user_type is not needed for the solution taken in this PR.

-- make sure that matches from previous test runs don't get in the way.
catMaybes
<$> forM foundDocs (\doc -> do
tidActual :: String <- doc %. "team" & asString
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: type annotation is redundant


-- the same goes for apps from the same team
postConnection mem1 app `bindResponse` \resp -> do
resp.status `shouldMatchInt` 404
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be 403, but not high prio issue.

Comment on lines 86 to 89
data UserType = UserTypeRegular | UserTypeApp | UserTypeBot
deriving (Eq, Show, Ord, Generic)
deriving (Arbitrary) via (GenericUniform UserType)
deriving (A.FromJSON, A.ToJSON, S.ToSchema) via (Schema UserType)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: maybe this can go back into the User module?

@fisx fisx marked this pull request as ready for review February 11, 2026 07:48
@fisx fisx requested review from a team as code owners February 11, 2026 07:48
@fisx fisx force-pushed the WPB-22549-connect-user-to-app branch from d90af36 to d344fec Compare February 11, 2026 08:07
@fisx fisx merged commit 8d56941 into develop Feb 11, 2026
9 of 10 checks passed
@fisx fisx deleted the WPB-22549-connect-user-to-app branch February 11, 2026 11:34
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