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

Add configuration files for campaigns #1258

Merged
merged 10 commits into from Jun 19, 2018
Merged

Add configuration files for campaigns #1258

merged 10 commits into from Jun 19, 2018

Conversation

gbirke
Copy link
Member

@gbirke gbirke commented Jun 11, 2018

This is an example on how to configure feature toggles and use them. The previous "campaign" feature for selecting the Confirmation page is replaced by the new A/B testing feature.

This is for https://phabricator.wikimedia.org/T196337

@gbirke

This comment has been minimized.

@timEulitz

This comment has been minimized.

@KaiNissen

This comment has been minimized.

@tzhelyazkova

This comment has been minimized.

@gbirke gbirke force-pushed the campaign-config branch 2 times, most recently from fb065b1 to d607834 Compare June 12, 2018 19:15
@gbirke gbirke changed the title [WIP] Add configuration files for campaigns Add configuration files for campaigns Jun 12, 2018
@gbirke gbirke changed the title Add configuration files for campaigns [WIP] Add configuration files for campaigns Jun 12, 2018
@gbirke gbirke force-pushed the campaign-config branch 2 times, most recently from 49396d4 to 1b62c09 Compare June 12, 2018 22:12
};

$container['campaign_config'] = function (): array {
// TODO move to CampaignFactory?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel like I understand the purpose / scope of this factory well enough to comment on this, maybe elaborate on this, possibly also in person.

@timEulitz
Copy link
Contributor

Setting aside the TODOs, I don't see any issues with the code in its current state.

On a very minor note, I think we agreed to phase out @author doc comments, though I don't fully recall how that conversation concluded. I don't personally care too much if you prefer leaving them in but just wanted to mention it quickly.

gbirke and others added 10 commits June 18, 2018 17:47
Add YAML files that show how we will define A/B testing campaigns in the
future.

Add a configuration class that can validate the tree structure of the
configuration and explains the meaning of each node.

TBD:
- Add validator class that validates not only completeness and
  well-formedness but also date format and default_group value
- Create command line validator and add it to CI
Add Campaign value object that holds information about running campaigns.
Add CampaignBuilder class to create Campaign objects from an array
configuration (that comes out of campaigns.yaml)
Use the Doorkeeper feature toggle library to convert each group of each
campaign into a feature. The default group of each campaign is always
enabled unconditionally. The other groups are converted into conditional
features based on date range of the campaign and a percentage.
Replace DonationConfirmationPageSelector with a method in the
ChoiceFactory, the central factory where all feature-toggle related
choices are made.

Fix conation confirmation template in test skin.
Removed DonationConfirmationPageSelector and TemplateTestCampaign.

DonationConfirmationPageSelector was superseded by
ChoiceFactory::getConfirmationPageTemplate.

TemplateTestCampaign was superseded by CampaignConfiguration.
Use an interface for different implementations of the feature toggle
library.
Add a DoorKeeper implementation.
Add a fixture implementation for testing. This allows you to test
specific code paths without having to add camapign configrurations
configuration, which is kind of environment-dependent.
Move the conditionals and branching out of FunFunFactory.

We're not catching any of the Exceptions that the loader might throw,
because we want to catch bad configuration in CI.
Change config schema for campaigns to allow for multiple files (for
testing, dev settings, etc.)

Fix issue where default Twig templates were not available on
confirmation page A/B test templates.

Add a cache to CampaignConfigurationLoader::loadCampaignConfiguration to
avoid having to parse and validate the configuration on every request.

Add cache to general caching and cache purging infrastructure.
Now the test code also reads the campaign configuration, being an erly
warning method.

Moved all campaign and feature-toggle related code in the FunFunFactory
to its shared object code instead of (mis)using Pimple for that.
@gbirke gbirke changed the title [WIP] Add configuration files for campaigns Add configuration files for campaigns Jun 18, 2018
@tzhelyazkova tzhelyazkova merged commit bc3e32f into master Jun 19, 2018
@tzhelyazkova tzhelyazkova deleted the campaign-config branch June 19, 2018 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants