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

Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice #1129

Merged
merged 10 commits into from
Jun 8, 2020

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Jun 4, 2020

Fixes https://github.com/zinfra/backend-issues/issues/1490

There was one Data.* module per column in the team_features table. That may make sense at some point in the distant future, but as long as all those columns are just booleans, having one haskell module to access any of them in a uniform way saves a lot of boilerplate.

I tried to sneak this into #1113, but it wasn't ready:

  • lots of tests failing.
  • stern does not have a validate-saml-emails switch.
  • stern should get a generic feature switch, where you can select a feature and a boolean.

fisx added 3 commits June 5, 2020 20:34
There was one Data.* module per column in the team_features table.
That may make sense at some point in the distant future, but as long
as all those columns are just booleans, having one haskell module to
access any of them in a uniform way saves a lot of boilerplate.
@fisx fisx force-pushed the fisx/streamline-team-feature-flags branch from 3c6eaee to dfff96d Compare June 5, 2020 19:51
@fisx fisx changed the title [WIP] Simplify team feature database interface. Simplify team feature database interface. Jun 5, 2020
@fisx
Copy link
Contributor Author

fisx commented Jun 5, 2020

lots of tests failing.

that was just one stupid typo.

stern should get a generic feature switch, where you can select a feature and a boolean.

done.

stern does not have a validate-saml-emails switch.

nothing needed to be done here: thanks to the generic end-points, stern needs no change any more if a new construct is added to TeamFeatureName. just recompile it.

not tested yet. i'm not sure any more how to run stern in an integration test setup.

@fisx fisx changed the title Simplify team feature database interface. Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice Jun 8, 2020
Comment on lines 32 to 37
-- | Is a given feature enabled or disabled? (If field is null: disabled.)
getFlag :: MonadClient m => TeamId -> TeamFeatureName -> m (Maybe TeamFeatureStatus)
getFlag tid feature = fmap toFlag <$> retry x1 (query1 (select feature) (params Quorum (Identity tid)))
where
toFlag (Identity Nothing) = TeamFeatureDisabled
toFlag (Identity (Just status)) = status
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't default to TeamFeatureDisabled here, for TeamSearchVisibility and SSO, the defaults come from galley config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, thanks! hum, annoying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, it was easier to fix than I thought, but it still was a bug: a2264fa

Now getFlag will also return Nothing for non-existant teams, but I think that was another bug: the situation that the entire row is missing should not be distinguished from the situation where the row is present, but the column is null. Which of the two happens arbitrarily depends on whether other team features have been touched for this team in the past.

services/galley/test/integration/API/Teams.hs Outdated Show resolved Hide resolved
tools/stern/src/Stern/API.hs Outdated Show resolved Hide resolved
@fisx fisx changed the title Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice [WIP] Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice Jun 8, 2020
@fisx fisx requested a review from akshaymankar June 8, 2020 09:27
@fisx fisx changed the title [WIP] Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice Simplify team feature database interface; add validate-saml-emails feature to stern/backoffice Jun 8, 2020
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

🚢 :shipit:

@fisx fisx merged commit 869c7ac into develop Jun 8, 2020
@fisx fisx deleted the fisx/streamline-team-feature-flags branch June 8, 2020 14:27
mheinzel pushed a commit that referenced this pull request Jun 8, 2020
…ature to stern/backoffice (#1129)

* Simplify team feature database interface.

There was one Data.* module per column in the team_features table.
That may make sense at some point in the distant future, but as long
as all those columns are just booleans, having one haskell module to
access any of them in a uniform way saves a lot of boilerplate.

* Use new feature flag types and make a generic end-point in stern.

* Add FUTUREWORKs.

Co-authored-by: Akshay Mankar <akshay@wire.com>
@fisx fisx mentioned this pull request Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants