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

Fix calendar integration setting in backoffice / stern #3761

Merged
merged 4 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5-internal/WPB-5541
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix calendar integration setting in backoffice / stern
3 changes: 2 additions & 1 deletion libs/wire-api/src/Wire/API/Team/Feature.hs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ instance ToSchema LockStatus where
element "unlocked" LockStatusUnlocked
]

instance S.ToParamSchema LockStatus
instance S.ToParamSchema LockStatus where
toParamSchema _ = mempty & S.type_ ?~ S.OpenApiString & S.enum_ ?~ ["locked", "unlocked"]

instance ToByteString LockStatus where
builder LockStatusLocked = "locked"
Expand Down
5 changes: 5 additions & 0 deletions tools/stern/src/Stern/API.hs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ sitemap' =
:<|> Named @"get-search-visibility" getSearchVisibility
:<|> Named @"put-search-visibility" setSearchVisibility
:<|> Named @"get-route-outlook-cal-config" (mkFeatureGetRoute @OutlookCalIntegrationConfig)
:<|> Named @"lock-unlock-route-outlook-cal-config" (mkFeatureLockUnlockRouteTrivialConfigNoTTL @OutlookCalIntegrationConfig)
:<|> Named @"put-route-outlook-cal-config" (mkFeaturePutRouteTrivialConfigNoTTL @OutlookCalIntegrationConfig)
:<|> Named @"get-team-invoice" getTeamInvoice
:<|> Named @"get-team-billing-info" getTeamBillingInfo
Expand Down Expand Up @@ -335,6 +336,10 @@ mkFeaturePutRouteTrivialConfigNoTTL ::
forall cfg. (MkFeaturePutConstraints cfg) => TeamId -> FeatureStatus -> Handler NoContent
mkFeaturePutRouteTrivialConfigNoTTL tid status = mkFeaturePutRouteTrivialConfig @cfg tid status Nothing

mkFeatureLockUnlockRouteTrivialConfigNoTTL ::
forall cfg. (MkFeaturePutConstraints cfg) => TeamId -> LockStatus -> Handler NoContent
mkFeatureLockUnlockRouteTrivialConfigNoTTL tid lstat = NoContent <$ Intra.setTeamFeatureLockStatus @cfg tid lstat

mkFeaturePutRouteTrivialConfigWithTTL ::
forall cfg. (MkFeaturePutConstraints cfg) => TeamId -> FeatureStatus -> FeatureTTLDays -> Handler NoContent
mkFeaturePutRouteTrivialConfigWithTTL tid status = mkFeaturePutRouteTrivialConfig @cfg tid status . Just
Expand Down
11 changes: 11 additions & 0 deletions tools/stern/src/Stern/API/Routes.hs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ type SternAPI =
:> Put '[JSON] NoContent
)
:<|> Named "get-route-outlook-cal-config" (MkFeatureGetRoute OutlookCalIntegrationConfig)
:<|> Named "lock-unlock-route-outlook-cal-config" (MkFeatureLockUnlockRouteTrivialConfigNoTTL OutlookCalIntegrationConfig)
:<|> Named "put-route-outlook-cal-config" (MkFeaturePutRouteTrivialConfigNoTTL OutlookCalIntegrationConfig)
:<|> Named
"get-team-invoice"
Expand Down Expand Up @@ -519,6 +520,16 @@ type MkFeaturePutRouteTrivialConfigWithTTL (feature :: Type) =
:> QueryParam' [Required, Strict, Description "team feature time to live, given in days, or 'unlimited' (default)."] "ttl" FeatureTTLDays
:> Put '[JSON] NoContent

type MkFeatureLockUnlockRouteTrivialConfigNoTTL (feature :: Type) =
Summary "Lock / unlock status for a given feature / team (en-/disable should happen in team settings)"
:> "teams"
:> Capture "tid" TeamId
:> "features"
:> FeatureSymbol feature
:> "lockOrUnlock"
:> QueryParam' [Required, Strict] "lock-status" LockStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

Our query parameters are mostly snake case, and sometimes camel case is used. But I haven't seen kebab cases so far. But I am ok with it here (it's just stern)

:> Put '[JSON] NoContent

type MkFeaturePutRoute (feature :: Type) =
Summary "Disable / enable feature flag for a given team"
:> "teams"
Expand Down
32 changes: 31 additions & 1 deletion tools/stern/src/Stern/Intra.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ module Stern.Intra
setBlacklistStatus,
getTeamFeatureFlag,
setTeamFeatureFlag,
setTeamFeatureLockStatus,
getTeamData,
getSearchVisibility,
setSearchVisibility,
Expand Down Expand Up @@ -86,12 +87,13 @@ import Data.Id
import Data.Int
import Data.List.Split (chunksOf)
import Data.Map qualified as Map
import Data.Proxy (Proxy (Proxy))
import Data.Qualified (qUnqualified)
import Data.Text (strip)
import Data.Text.Encoding (decodeUtf8, encodeUtf8)
import Data.Text.Lazy (pack)
import Data.Text.Lazy.Encoding qualified as TL
import GHC.TypeLits (KnownSymbol)
import GHC.TypeLits (KnownSymbol, symbolVal)
import Imports
import Network.HTTP.Types (urlEncode)
import Network.HTTP.Types.Method
Expand Down Expand Up @@ -571,6 +573,34 @@ setTeamFeatureFlag tid status = do
where
daysLimit = 2000

setTeamFeatureLockStatus ::
forall cfg.
( KnownSymbol (Public.FeatureSymbol cfg)
) =>
TeamId ->
LockStatus ->
Handler ()
setTeamFeatureLockStatus tid lstat = do
info $ msg ("Setting lock status: " <> show (symbolVal (Proxy @(Public.FeatureSymbol cfg)), lstat))
gly <- view galley
fromResponseBody <=< catchRpcErrors $
rpc'
"galley"
gly
( method PUT
. Bilge.paths
[ "i",
"teams",
toByteString' tid,
"features",
Public.featureNameBS @cfg,
toByteString' lstat
]
)
where
fromResponseBody :: Response (Maybe LByteString) -> Handler ()
fromResponseBody resp = parseResponse (mkError status502 "bad-upstream") resp

getSearchVisibility :: TeamId -> Handler TeamSearchVisibilityView
getSearchVisibility tid = do
info $ msg "Getting TeamSearchVisibilityView value"
Expand Down
66 changes: 66 additions & 0 deletions tools/stern/test/integration/API.hs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ tests s =
test s "/teams/:tid/features/mls" $ testFeatureConfig @MLSConfig,
test s "GET /teams/:tid/features/classifiedDomains" $ testGetFeatureConfig @ClassifiedDomainsConfig (Just FeatureStatusEnabled),
test s "GET /teams/:tid/features/outlookCalIntegration" $ testFeatureStatus @OutlookCalIntegrationConfig,
test s "PUT /teams/:tid/features/outlookCalIntegration{,'?lockOrUnlock'}" $ testFeatureStatusWithLock @OutlookCalIntegrationConfig,
test s "GET /i/consent" testGetConsentLog,
test s "GET /teams/:id" testGetTeamInfo,
test s "GET i/user/meta-info?id=..." testGetUserMetaInfo,
Expand Down Expand Up @@ -337,6 +338,46 @@ testFeatureStatusOptTtl mTtl = do
cfg' <- getFeatureConfig @cfg tid
liftIO $ wsStatus cfg' @?= newStatus

testFeatureStatusWithLock ::
forall cfg.
( KnownSymbol (FeatureSymbol cfg),
ToSchema cfg,
Typeable cfg,
IsFeatureConfig cfg,
Eq cfg,
Show cfg
) =>
TestM ()
testFeatureStatusWithLock = do
let mTtl = Nothing -- this function can become a variant of `testFeatureStatusOptTtl` if we need one.
(_, tid, _) <- createTeamWithNMembers 10
getFeatureConfig @cfg tid >>= \cfg -> liftIO $ do
cfg @?= defFeatureStatus @cfg
-- if either of these two lines fails, it's probably because the default is surprising.
-- in that case, make the text more flexible.
wsLockStatus cfg @?= LockStatusLocked
wsStatus cfg @?= FeatureStatusDisabled

void $ putFeatureStatusLock @cfg tid LockStatusUnlocked mTtl
getFeatureConfig @cfg tid >>= \cfg -> liftIO $ do
wsLockStatus cfg @?= LockStatusUnlocked
wsStatus cfg @?= FeatureStatusDisabled

void $ putFeatureStatus @cfg tid FeatureStatusEnabled Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

Testing to en-/disable the feature isn't really needed IMO, but it also doesn't hurt.

getFeatureConfig @cfg tid >>= \cfg -> liftIO $ do
wsLockStatus cfg @?= LockStatusUnlocked
wsStatus cfg @?= FeatureStatusEnabled

void $ putFeatureStatusLock @cfg tid LockStatusLocked mTtl
getFeatureConfig @cfg tid >>= \cfg -> liftIO $ do
wsLockStatus cfg @?= LockStatusLocked
wsStatus cfg @?= FeatureStatusDisabled

void $ putFeatureStatusLock @cfg tid LockStatusUnlocked mTtl
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we implemented the team features so that if they are locked, they are reset to their default, which would be "disabled", no? I am surprised that this test passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if locked, status is the default. but if unlocked, it switches back to whatever it was during the last unlock window. we can debate whether this is a good idea, but last time we did we concluded that it's nice for the admin to not lose settings if they forget to update their cc info.

getFeatureConfig @cfg tid >>= \cfg -> liftIO $ do
wsLockStatus cfg @?= LockStatusUnlocked
wsStatus cfg @?= FeatureStatusEnabled

testGetConsentLog :: TestM ()
testGetConsentLog = do
(_, email) <- randomEmailUser
Expand Down Expand Up @@ -619,6 +660,31 @@ putFeatureStatus tid status mTtl = do
mkTtlQueryParam :: Maybe FeatureTTL -> Request -> Request
mkTtlQueryParam = maybe id (queryItem "ttl" . toByteString')

putFeatureStatusLock ::
forall cfg.
( KnownSymbol (FeatureSymbol cfg),
ToSchema cfg,
Typeable cfg,
IsFeatureConfig cfg
) =>
TeamId ->
LockStatus ->
Maybe FeatureTTL ->
TestM ResponseLBS
putFeatureStatusLock tid lockStatus mTtl = do
s <- view tsStern
put
( s
. paths ["teams", toByteString' tid, "features", Public.featureNameBS @cfg, "lockOrUnlock"]
. queryItem "lock-status" (toByteString' lockStatus)
. mkTtlQueryParam mTtl
. contentJson
. expect2xx
)
where
mkTtlQueryParam :: Maybe FeatureTTL -> Request -> Request
mkTtlQueryParam = maybe id (queryItem "ttl" . toByteString')

putFeatureConfig ::
forall cfg.
( KnownSymbol (FeatureSymbol cfg),
Expand Down
Loading