From 5c757f0e0005aec28119d551cd765b194a781d53 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Fri, 1 Jul 2016 10:38:27 -0700 Subject: [PATCH 01/74] Pass engagementButtonLabel to showHeartbeat properly. --- .../control/js/components/RecipePreview.js | 5 +- .../selfrepair/static/js/normandy_driver.js | 87 ++++++++++--------- normandy/selfrepair/static/js/self_repair.js | 12 ++- .../static/js/self_repair_runner.js | 29 +++---- .../selfrepair/tests/test_normandy_driver.js | 38 ++++++++ webpack.config.js | 1 + 6 files changed, 111 insertions(+), 61 deletions(-) create mode 100644 normandy/selfrepair/tests/test_normandy_driver.js diff --git a/normandy/control/static/control/js/components/RecipePreview.js b/normandy/control/static/control/js/components/RecipePreview.js index 3302d14b..e457efda 100644 --- a/normandy/control/static/control/js/components/RecipePreview.js +++ b/normandy/control/static/control/js/components/RecipePreview.js @@ -2,6 +2,7 @@ import React, { PropTypes as pt } from 'react'; import classNames from 'classnames'; import composeRecipeContainer from './RecipeContainer.js'; import { runRecipe } from '../../../../../selfrepair/static/js/self_repair_runner.js'; +import NormandyDriver from '../../../../../selfrepair/static/js/normandy_driver.js'; class RecipePreview extends React.Component { propTypes = { @@ -29,13 +30,15 @@ class RecipePreview extends React.Component { attemptPreview() { const { recipe } = this.props; const { recipeAttempted } = this.state; + const driver = new NormandyDriver(); + driver.registerCallbacks(); if (recipe && !recipeAttempted) { this.setState({ recipeAttempted: true, }); - runRecipe(recipe, { testing: true }).then(() => { + runRecipe(recipe, driver, { testing: true }).then(() => { this.setState({ recipeExecuted: true, }); diff --git a/normandy/selfrepair/static/js/normandy_driver.js b/normandy/selfrepair/static/js/normandy_driver.js index fe985338..dce27765 100644 --- a/normandy/selfrepair/static/js/normandy_driver.js +++ b/normandy/selfrepair/static/js/normandy_driver.js @@ -40,21 +40,40 @@ Object.assign(LocalStorage.prototype, { /** * Implementation of the Normandy driver. */ -const Normandy = { - locale: document.documentElement.dataset.locale || navigator.language, +export default class NormandyDriver { + constructor(uitour=Mozilla.UITour) { + this._uitour = uitour; + } - _testingOverride: false, + _heartbeatCallbacks = []; + registerCallbacks() { + // Trigger heartbeat callbacks when the UITour tells us that Heartbeat + // happened. + this._uitour.observe((eventName, data) => { + if (eventName.startsWith('Heartbeat:')) { + const flowId = data.flowId; + const croppedEventName = eventName.slice(10); // Chop off "Heartbeat:" + if (flowId in this._heartbeatCallbacks) { + this._heartbeatCallbacks[flowId](croppedEventName, data); + } + } + }); + } + + locale = document.documentElement.dataset.locale || navigator.language; + + _testingOverride = false; get testing() { return this._testingOverride || new URL(window.location.href).searchParams.has('testing'); - }, + } set testing(value) { this._testingOverride = value; - }, + } - _location: { countryCode: null }, + _location = { countryCode: null }; location() { return Promise.resolve(this._location); - }, + } log(message, level = 'debug') { if (level === 'debug' && !this.testing) { @@ -64,15 +83,15 @@ const Normandy = { } else { console.log(message); } - }, + } uuid() { return uuid.v4(); - }, + } createStorage(prefix) { return new LocalStorage(prefix); - }, + } client() { return new Promise(resolve => { @@ -80,7 +99,7 @@ const Normandy = { plugins: {}, }; - // Populate plugin info. + // Populate plugin info. for (const plugin of navigator.plugins) { client.plugins[plugin.name] = { name: plugin.name, @@ -109,7 +128,7 @@ const Normandy = { let retrievedConfigs = 0; const wantedConfigNames = Object.keys(wantedConfigs); wantedConfigNames.forEach(configName => { - Mozilla.UITour.getConfiguration(configName, data => { + this._uitour.getConfiguration(configName, data => { wantedConfigs[configName](data); retrievedConfigs++; if (retrievedConfigs >= wantedConfigNames.length) { @@ -118,7 +137,7 @@ const Normandy = { }); }); }); - }, + } saveHeartbeatFlow(data) { if (this.testing) { @@ -134,46 +153,32 @@ const Normandy = { 'Content-Type': 'application/json', }, }); - }, + } - heartbeatCallbacks: [], + heartbeatCallbacks = []; showHeartbeat(options) { return new Promise(resolve => { const emitter = new EventEmitter(); this.heartbeatCallbacks[options.flowId] = (eventName, data) => emitter.emit(eventName, data); - // Positional arguments are overridden by the final options - // argument, but they're still required so we pass them anyway. - Mozilla.UITour.showHeartbeat( - options.message, - options.thanksMessage, - options.flowId, - options.postAnswerUrl, - options.learnMoreMessage, - options.learnMoreUrl, + // Positional arguments are overridden by the final options + // argument, but they're still required so we pass them anyway. + this._uitour.showHeartbeat( + options.message, + options.thanksMessage, + options.flowId, + options.postAnswerUrl, + options.learnMoreMessage, + options.learnMoreUrl, { + engagementButtonLabel: options.engagementButtonLabel, surveyId: options.surveyId, surveyVersion: options.surveyVersion, testing: options.testing, } - ); + ); resolve(emitter); }); - }, -}; - - -// Trigger heartbeat callbacks when the UITour tells us that Heartbeat -// happened. -Mozilla.UITour.observe((eventName, data) => { - if (eventName.startsWith('Heartbeat:')) { - const flowId = data.flowId; - const croppedEventName = eventName.slice(10); // Chop off "Heartbeat:" - if (flowId in Normandy.heartbeatCallbacks) { - Normandy.heartbeatCallbacks[flowId](croppedEventName, data); - } } -}); - -export default Normandy; +} diff --git a/normandy/selfrepair/static/js/self_repair.js b/normandy/selfrepair/static/js/self_repair.js index e54c869a..d794f73c 100644 --- a/normandy/selfrepair/static/js/self_repair.js +++ b/normandy/selfrepair/static/js/self_repair.js @@ -1,16 +1,20 @@ -import Normandy from './normandy_driver.js'; +import Mozilla from './uitour.js'; +import NormandyDriver from './normandy_driver.js'; import { fetchRecipes, filterContext, doesRecipeMatch, runRecipe } from './self_repair_runner.js'; +const driver = new NormandyDriver(Mozilla.UITour); +driver.registerCallbacks(); + // Actually fetch and run the recipes. fetchRecipes().then(recipes => { - filterContext().then(context => { + filterContext(driver).then(context => { // Update Normandy driver with user's country. - Normandy._location.countryCode = context.normandy.country; + driver._location.countryCode = context.normandy.country; for (const recipe of recipes) { doesRecipeMatch(recipe, context).then(([, match]) => { if (match) { - runRecipe(recipe).catch(err => { + runRecipe(recipe, driver).catch(err => { console.error(err); }); } diff --git a/normandy/selfrepair/static/js/self_repair_runner.js b/normandy/selfrepair/static/js/self_repair_runner.js index 5b2d7fb7..7c6c5099 100644 --- a/normandy/selfrepair/static/js/self_repair_runner.js +++ b/normandy/selfrepair/static/js/self_repair_runner.js @@ -1,4 +1,3 @@ -import Normandy from './normandy_driver.js'; import uuid from 'node-uuid'; import jexl from 'jexl'; @@ -78,22 +77,22 @@ export function fetchRecipes() { * Fetch client information from the Normandy server and the driver. * @promise Resolves with an object containing client info. */ -function classifyClient() { +function classifyClient(driver) { const { classifyUrl } = document.documentElement.dataset; const headers = { Accept: 'application/json' }; const classifyXhr = fetch(classifyUrl, { headers }) .then(response => response.json()) .then(client => client); - return Promise.all([classifyXhr, Normandy.client()]) - .then(([classification, client]) => { - // Parse request time - classification.request_time = new Date(classification.request_time); + return Promise.all([classifyXhr, driver.client()]) + .then(([classification, client]) => { + // Parse request time + classification.request_time = new Date(classification.request_time); - return Object.assign({ - locale: Normandy.locale, - }, classification, client); - }); + return Object.assign({ + locale: driver.locale, + }, classification, client); + }); } @@ -103,13 +102,13 @@ function classifyClient() { * @param {Recipe} recipe - Recipe retrieved from the server. * @promise Resolves once the action has executed. */ -export function runRecipe(recipe, options = {}) { +export function runRecipe(recipe, driver, options = {}) { return loadAction(recipe).then(Action => { if (options.testing !== undefined) { - Normandy.testing = options.testing; + driver.testing = options.testing; } - return new Action(Normandy, recipe).execute(); + return new Action(driver, recipe).execute(); }); } @@ -118,8 +117,8 @@ export function runRecipe(recipe, options = {}) { * Generate a context object for JEXL filter expressions. * @return {object} */ -export function filterContext() { - return classifyClient() +export function filterContext(driver) { + return classifyClient(driver) .then(classifiedClient => ({ normandy: classifiedClient, })); diff --git a/normandy/selfrepair/tests/test_normandy_driver.js b/normandy/selfrepair/tests/test_normandy_driver.js new file mode 100644 index 00000000..19deaea7 --- /dev/null +++ b/normandy/selfrepair/tests/test_normandy_driver.js @@ -0,0 +1,38 @@ +import NormandyDriver from '../static/js/normandy_driver.js'; + +describe('Normandy Driver', () => { + describe('showHeartbeat', () => { + it('should pass all the required arguments to the UITour helper', async function() { + let uitour = jasmine.createSpyObj('uitour', ['showHeartbeat']); + let driver = new NormandyDriver(uitour); + let options = { + message: 'testMessage', + thanksMessage: 'testThanks', + flowId: 'testFlowId', + postAnswerUrl: 'testPostAnswerUrl', + learnMoreMessage: 'testLearnMoreMessage', + learnMoreUrl: 'testLearnMoreUrl', + engagementButtonLabel: 'testEngagementButtonLabel', + surveyId: 'testSurveyId', + surveyVersion: 'testSurveyVersion', + testing: true + }; + + await driver.showHeartbeat(options); + expect(uitour.showHeartbeat).toHaveBeenCalledWith( + options.message, + options.thanksMessage, + options.flowId, + options.postAnswerUrl, + options.learnMoreMessage, + options.learnMoreUrl, + jasmine.objectContaining({ + engagementButtonLabel: options.engagementButtonLabel, + surveyId: options.surveyId, + surveyVersion: options.surveyVersion, + testing: options.testing, + }), + ); + }); + }); +}); diff --git a/webpack.config.js b/webpack.config.js index a8144e12..e892ba8e 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -20,6 +20,7 @@ module.exports = [ './normandy/selfrepair/static/js/self_repair', ], control: [ + 'babel-polyfill', './normandy/control/static/control/js/index', './normandy/control/static/control/admin/sass/control.scss', './node_modules/font-awesome/scss/font-awesome.scss', From fa7050316962fbfd83115f308ac80411941c8626 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Fri, 1 Jul 2016 10:39:38 -0700 Subject: [PATCH 02/74] Refactor LocalStorage to ES6 class syntax. --- .../selfrepair/static/js/normandy_driver.js | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/normandy/selfrepair/static/js/normandy_driver.js b/normandy/selfrepair/static/js/normandy_driver.js index dce27765..0d134d5f 100644 --- a/normandy/selfrepair/static/js/normandy_driver.js +++ b/normandy/selfrepair/static/js/normandy_driver.js @@ -4,38 +4,31 @@ import uuid from 'node-uuid'; /** * Storage class that uses window.localStorage as it's backing store. - * @param {string} prefix Prefix to append to all incoming keys. */ -function LocalStorage(prefix) { - this.prefix = prefix; -} +class LocalStorage { + /** + * @param {string} prefix Prefix to append to all incoming keys. + */ + constructor(prefix) { + this.prefix = prefix; + } -Object.assign(LocalStorage.prototype, { _makeKey(key) { return `${this.prefix}-${key}`; - }, + } - getItem(key) { - return new Promise(resolve => { - resolve(localStorage.getItem(this._makeKey(key))); - }); - }, + async getItem(key) { + return localStorage.getItem(this._makeKey(key)); + } - setItem(key, value) { - return new Promise(resolve => { - localStorage.setItem(this._makeKey(key), value); - resolve(); - }); - }, - - removeItem(key) { - return new Promise(resolve => { - localStorage.removeItem(this._makeKey(key)); - resolve(); - }); - }, -}); + async setItem(key, value) { + return localStorage.setItem(this._makeKey(key), value); + } + async removeItem(key) { + return localStorage.removeItem(this._makeKey(key)); + } +} /** * Implementation of the Normandy driver. From 827439e4656676575f0db0a1e4f36dd5188297a4 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Wed, 6 Jul 2016 21:58:55 -0700 Subject: [PATCH 03/74] Lint fixes. --- normandy/selfrepair/static/js/normandy_driver.js | 4 ++-- normandy/selfrepair/tests/test_normandy_driver.js | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/normandy/selfrepair/static/js/normandy_driver.js b/normandy/selfrepair/static/js/normandy_driver.js index 0d134d5f..ff49e9bb 100644 --- a/normandy/selfrepair/static/js/normandy_driver.js +++ b/normandy/selfrepair/static/js/normandy_driver.js @@ -34,8 +34,8 @@ class LocalStorage { * Implementation of the Normandy driver. */ export default class NormandyDriver { - constructor(uitour=Mozilla.UITour) { - this._uitour = uitour; + constructor(uitour = Mozilla.UITour) { + this._uitour = uitour; } _heartbeatCallbacks = []; diff --git a/normandy/selfrepair/tests/test_normandy_driver.js b/normandy/selfrepair/tests/test_normandy_driver.js index 19deaea7..08f4c043 100644 --- a/normandy/selfrepair/tests/test_normandy_driver.js +++ b/normandy/selfrepair/tests/test_normandy_driver.js @@ -2,10 +2,10 @@ import NormandyDriver from '../static/js/normandy_driver.js'; describe('Normandy Driver', () => { describe('showHeartbeat', () => { - it('should pass all the required arguments to the UITour helper', async function() { - let uitour = jasmine.createSpyObj('uitour', ['showHeartbeat']); - let driver = new NormandyDriver(uitour); - let options = { + it('should pass all the required arguments to the UITour helper', async () => { + const uitour = jasmine.createSpyObj('uitour', ['showHeartbeat']); + const driver = new NormandyDriver(uitour); + const options = { message: 'testMessage', thanksMessage: 'testThanks', flowId: 'testFlowId', @@ -15,7 +15,7 @@ describe('Normandy Driver', () => { engagementButtonLabel: 'testEngagementButtonLabel', surveyId: 'testSurveyId', surveyVersion: 'testSurveyVersion', - testing: true + testing: true, }; await driver.showHeartbeat(options); From 3ffa2daeb553f070828634c75d80c256271813e1 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Wed, 6 Jul 2016 22:00:08 -0700 Subject: [PATCH 04/74] Review fixes. --- normandy/selfrepair/static/js/self_repair.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/normandy/selfrepair/static/js/self_repair.js b/normandy/selfrepair/static/js/self_repair.js index d794f73c..c6319b68 100644 --- a/normandy/selfrepair/static/js/self_repair.js +++ b/normandy/selfrepair/static/js/self_repair.js @@ -1,8 +1,7 @@ -import Mozilla from './uitour.js'; import NormandyDriver from './normandy_driver.js'; import { fetchRecipes, filterContext, doesRecipeMatch, runRecipe } from './self_repair_runner.js'; -const driver = new NormandyDriver(Mozilla.UITour); +const driver = new NormandyDriver(); driver.registerCallbacks(); // Actually fetch and run the recipes. From b52b15a759a4d65d815fca8b2b0b591d3b7eb0cf Mon Sep 17 00:00:00 2001 From: Mike Cooper Date: Thu, 7 Jul 2016 16:10:13 -0700 Subject: [PATCH 05/74] Fix errors introduced during linting Fixes bug 1271278. --- .../control/tests/actions/controlActions.js | 17 +++++++++++++++++ normandy/recipes/static/js/arguments_editor.js | 10 +++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/normandy/control/tests/actions/controlActions.js b/normandy/control/tests/actions/controlActions.js index 6e1510f6..1cfb899d 100644 --- a/normandy/control/tests/actions/controlActions.js +++ b/normandy/control/tests/actions/controlActions.js @@ -124,4 +124,21 @@ describe('controlApp Actions', () => { expect(fetchMock.calls('/api/v1/recipe/1/').length).toEqual(1); }); }); + + + describe('showNotification', () => { + it('automatically dismisses notifications after 10 seconds', async () => { + jasmine.clock().install(); + + const notification = { messageType: 'success', message: 'message' }; + await store.dispatch(actionTypes.showNotification(notification)); + + const dismissAction = actionTypes.dismissNotification(notification.id); + expect(store.getActions()).not.toContain(dismissAction); + jasmine.clock().tick(10001); + expect(store.getActions()).toContain(dismissAction); + + jasmine.clock().uninstall(); + }); + }); }); diff --git a/normandy/recipes/static/js/arguments_editor.js b/normandy/recipes/static/js/arguments_editor.js index 94dab202..e59735a7 100644 --- a/normandy/recipes/static/js/arguments_editor.js +++ b/normandy/recipes/static/js/arguments_editor.js @@ -10,18 +10,18 @@ var $editorElement = $('
'); var editor = null; - // Insert editor next to field. + // Insert editor next to field. $editorElement.insertBefore($argumentsJson); - // Update the backing textarea before submitting. + // Update the backing textarea before submitting. $actionSelect.parents('form').submit(function storeValue() { if (editor !== null) { $argumentsJson.val(JSON.stringify(editor.getValue())); } }); - // When the action type changes, remove the existing editor and - // create a new one. + // When the action type changes, remove the existing editor and + // create a new one. $actionSelect.change(function handleChange() { var actionName; if (editor !== null) { @@ -33,7 +33,7 @@ return; } - $actionSelect.find('option:selected').text(); + actionName = $actionSelect.find('option:selected').text(); $.getJSON('/api/v1/action/' + actionName + '/', function processAction(action) { var data; editor = new JSONEditor($editorElement[0], { From f19ef67f523794deec7b79f4fbae39daaa0d46a6 Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Wed, 6 Jul 2016 09:34:41 -0400 Subject: [PATCH 06/74] Add notifications for client-side validation errors & deleting recipes Fixes bug 1283937 --- .../control/static/control/js/actions/ControlActions.js | 2 ++ .../control/static/control/js/components/RecipeForm.js | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/normandy/control/static/control/js/actions/ControlActions.js b/normandy/control/static/control/js/actions/ControlActions.js index f093c52b..544b7e86 100644 --- a/normandy/control/static/control/js/actions/ControlActions.js +++ b/normandy/control/static/control/js/actions/ControlActions.js @@ -95,6 +95,8 @@ const apiRequestMap = { settings: { method: 'DELETE', }, + successNotification: 'Recipe deleted.', + errorNotification: 'Error deleting recipe.', }; }, }; diff --git a/normandy/control/static/control/js/components/RecipeForm.js b/normandy/control/static/control/js/components/RecipeForm.js index 0a60e5fc..c8cc79cc 100644 --- a/normandy/control/static/control/js/components/RecipeForm.js +++ b/normandy/control/static/control/js/components/RecipeForm.js @@ -4,7 +4,8 @@ import { push } from 'react-router-redux'; import { destroy, reduxForm, getValues } from 'redux-form'; import jexl from 'jexl'; -import { makeApiRequest, recipeUpdated, recipeAdded } from '../actions/ControlActions.js'; +import { makeApiRequest, recipeUpdated, recipeAdded, showNotification } + from '../actions/ControlActions.js'; import composeRecipeContainer from './RecipeContainer.js'; import ActionForm from './ActionForm.js'; import CheckboxField from './form_fields/CheckboxField.js'; @@ -65,6 +66,10 @@ export class RecipeForm extends React.Component { return this.validateForm(combinedFormValues) .catch(() => { + dispatch(showNotification({ + messageType: 'error', + message: 'Recipe cannot be saved. Please correct any errors listed in the form below.', + })); throw { filter_expression: 'Invalid Expression', }; From 25bf1191e8ac5338c19b20f872e4d1218c878ec4 Mon Sep 17 00:00:00 2001 From: Chris Hartjes Date: Mon, 11 Jul 2016 13:18:08 -0400 Subject: [PATCH 07/74] Initial commit of contract tests for the recipe server API --- CONTRACT-TESTS.md | 20 ++++++++++++++++++++ conftest.py | 16 ++++++++++++++++ manifest.ini | 2 ++ test-requirements.txt | 5 +++++ 4 files changed, 43 insertions(+) create mode 100644 CONTRACT-TESTS.md create mode 100644 conftest.py create mode 100644 manifest.ini create mode 100644 test-requirements.txt diff --git a/CONTRACT-TESTS.md b/CONTRACT-TESTS.md new file mode 100644 index 00000000..b1b39110 --- /dev/null +++ b/CONTRACT-TESTS.md @@ -0,0 +1,20 @@ +# API Contract Tests + +It's highly recommended to use [Virtualenv](https://virtualenv.pypa.io/en/latest/) +in order to have an isolated environment. + +From the root directory of the project + +1. `virtualenv venv` +2. `source venv/bin/activate` to turn on the virtualenv +2. `./venv/bin/pip install -r test-requirements.txt` + + +### Schema Check Tests + +These tests are designed to look for changes to the recipe server API that are +not expected. + +`py.test --env= contract-tests/` + +where `` is one of `dev`, `stage`, or `prod` diff --git a/conftest.py b/conftest.py new file mode 100644 index 00000000..4be86535 --- /dev/null +++ b/conftest.py @@ -0,0 +1,16 @@ +# Configuration file for running our tests +import pytest + + +def pytest_addoption(parser): + parser.addoption( + "--env", + action="store", + required=True, + help="Choose a test environment: dev, stage, or prod", + ) + + +@pytest.fixture +def env(request): + return request.config.getoption("--env") diff --git a/manifest.ini b/manifest.ini new file mode 100644 index 00000000..621fb13c --- /dev/null +++ b/manifest.ini @@ -0,0 +1,2 @@ +[dev] +api_root = https://normandy.dev.mozaws.net/api/v1 diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 00000000..ebccf2e9 --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1,5 @@ +configparser +flake8 +jsonschema +pytest +requests From 617751ae34b5d1ef468a92dc92f1cce352348585 Mon Sep 17 00:00:00 2001 From: Chris Hartjes Date: Mon, 11 Jul 2016 13:20:10 -0400 Subject: [PATCH 08/74] Add in API tests --- contract-tests/test_api.py | 160 +++++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 contract-tests/test_api.py diff --git a/contract-tests/test_api.py b/contract-tests/test_api.py new file mode 100644 index 00000000..e2e9637c --- /dev/null +++ b/contract-tests/test_api.py @@ -0,0 +1,160 @@ +import configparser +import jsonschema +import pytest +import requests + + +@pytest.fixture +def conf(): + config = configparser.ConfigParser() + config.read('manifest.ini') + return config + + +def test_recipes(conf, env): + r = requests.get(conf.get(env, 'api_root') + '/recipe') + response = r.json() + expected_fields = [ + 'id', + 'last_updated', + 'name', + 'enabled', + 'revision_id', + 'action', + 'arguments', + 'filter_expression', + 'current_approval_request', + 'approval', + 'is_approved' + ] + + expected_arguments = [ + 'defaults', + 'surveyId', + 'surveys' + ] + + expected_survey_fields = [ + 'message', + 'thanksMessage', + 'engagementButtonLabel', + 'title', + 'postAnswerUrl', + 'weight', + 'learnMoreUrl', + 'learnMoreMessage', + ] + + expected_default_fields = [ + 'message', + 'thanksMessage', + 'engagementButtonLabel', + 'postAnswerUrl', + 'learnMoreUrl', + 'learnMoreMessage' + ] + + # Verify we have at least one response + assert len(response) >= 1 + + # Take a look at the first response and look for the expected fields + record = response[0] + for field in expected_fields: + assert field in record + + # Do the arguments look right? + for argument in expected_arguments: + assert argument in record['arguments'] + + # Do the defaults look right? + for field in record['arguments']['defaults']: + assert field in expected_default_fields + + # Do survey fields look right? Look at the first one + for field in record['arguments']['surveys'][0]: + assert field in expected_survey_fields + + +def filter_show_heartbeat(record): + return record['name'] == 'show-heartbeat' + + +def filter_console_log(record): + return record['name'] == 'console-log' + + +def test_expected_action_types(conf, env): + r = requests.get(conf.get(env, 'api_root') + '/action') + response = r.json() + + # Verify we have at least one response and then grab the first record + assert len(response) >= 1 + + # Make sure we only have expected action types + expected_records = ['console-log', 'show-heartbeat'] + + for record in response: + assert record['name'] in expected_records + + +def test_console_log(conf, env): + r = requests.get(conf.get(env, 'api_root') + '/action') + response = r.json() + + # Verify we have at least one response and then grab the first record + assert len(response) >= 1 + + # Look for any console-log actions + cl_records = list(filter(filter_console_log, response)) + + if len(cl_records) == 0: + pytest.skip('No console-log actions found') + else: + console_log_test(cl_records[0]) + + +def test_show_heartbeat(conf, env): + r = requests.get(conf.get(env, 'api_root') + '/action') + response = r.json() + + # Verify we have at least one response and then grab the first record + assert len(response) >= 1 + + # Let's find at least one record that is a 'show-heartbeat' + sh_records = list(filter(filter_show_heartbeat, response)) + + if len(sh_records) == 0: + pytest.skip('No show-heartbeat actions found') + else: + show_hearbeat_test(sh_records[0]) + + +def show_hearbeat_test(record): + expected_action_fields = [ + 'name', + 'implementation_url', + 'arguments_schema' + ] + for field in record: + assert field in expected_action_fields + + # Do we have a valid schema for 'arguments_schema'? + r = requests.get(record['arguments_schema']['$schema']) + schema = r.json() + assert jsonschema.validate(record['arguments_schema'], schema) is None + + +def console_log_test(record): + # Does an 'action' have all the required fields? + expected_action_fields = [ + 'name', + 'implementation_url', + 'arguments_schema' + ] + for field in record: + assert field in expected_action_fields + + # Do we have a valid schema for 'arguments_schema'? + r = requests.get(record['arguments_schema']['$schema']) + schema = r.json() + assert jsonschema.validate(record['arguments_schema'], schema) is None From b1451daff1a78c034b4448f468809eb3f1937b17 Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Tue, 12 Jul 2016 07:15:07 -0400 Subject: [PATCH 09/74] Fix prop values in ControlApp --- .../control/js/components/ControlApp.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/normandy/control/static/control/js/components/ControlApp.js b/normandy/control/static/control/js/components/ControlApp.js index ada389ed..c05367e4 100644 --- a/normandy/control/static/control/js/components/ControlApp.js +++ b/normandy/control/static/control/js/components/ControlApp.js @@ -1,22 +1,28 @@ -import React from 'react'; +import React, { PropTypes as pt } from 'react'; import Header from './Header.js'; import Notifications from './Notifications.js'; -export default function ControlApp() { +export default function ControlApp({ children, location, routes, params }) { return (
{ - React.Children.map(this.props.children, child => React.cloneElement(child)) + React.Children.map(children, child => React.cloneElement(child)) }
); } +ControlApp.propTypes = { + children: pt.object.isRequired, + location: pt.object.isRequired, + routes: pt.object.isRequired, + params: pt.object.isRequired, +}; From 3ed2adc01cce2bf8d384d898dfa65cf819a88a7c Mon Sep 17 00:00:00 2001 From: Chris Hartjes Date: Tue, 12 Jul 2016 14:57:21 -0400 Subject: [PATCH 10/74] Cleaned up issues with dependencies needed for API tests, created documentation in the docs/ directory, altered circle.yml file to specify which tests to run with pytest --- CONTRACT-TESTS.md | 20 ------------ circle.yml | 2 +- contract-tests/test_api.py | 64 -------------------------------------- docs/dev/api-tests.rst | 13 ++++++++ requirements.txt | 2 ++ test-requirements.txt | 5 --- 6 files changed, 16 insertions(+), 90 deletions(-) delete mode 100644 CONTRACT-TESTS.md create mode 100644 docs/dev/api-tests.rst delete mode 100644 test-requirements.txt diff --git a/CONTRACT-TESTS.md b/CONTRACT-TESTS.md deleted file mode 100644 index b1b39110..00000000 --- a/CONTRACT-TESTS.md +++ /dev/null @@ -1,20 +0,0 @@ -# API Contract Tests - -It's highly recommended to use [Virtualenv](https://virtualenv.pypa.io/en/latest/) -in order to have an isolated environment. - -From the root directory of the project - -1. `virtualenv venv` -2. `source venv/bin/activate` to turn on the virtualenv -2. `./venv/bin/pip install -r test-requirements.txt` - - -### Schema Check Tests - -These tests are designed to look for changes to the recipe server API that are -not expected. - -`py.test --env= contract-tests/` - -where `` is one of `dev`, `stage`, or `prod` diff --git a/circle.yml b/circle.yml index 9d15cd7e..56225729 100644 --- a/circle.yml +++ b/circle.yml @@ -35,7 +35,7 @@ test: docker run --net host -e DJANGO_CONFIGURATION=Test -v $CIRCLE_TEST_REPORTS:/test_artifacts -v ~/cache/GeoLite2-Country.mmdb:/app/GeoLite2-Country.mmdb - normandy:build py.test --junitxml=/test_artifacts/pytest.xml + normandy:build py.test --junitxml=/test_artifacts/pytest.xml normandy/ # Start Karma test server, and run them in Firefox - > ( diff --git a/contract-tests/test_api.py b/contract-tests/test_api.py index e2e9637c..65a28943 100644 --- a/contract-tests/test_api.py +++ b/contract-tests/test_api.py @@ -11,70 +11,6 @@ def conf(): return config -def test_recipes(conf, env): - r = requests.get(conf.get(env, 'api_root') + '/recipe') - response = r.json() - expected_fields = [ - 'id', - 'last_updated', - 'name', - 'enabled', - 'revision_id', - 'action', - 'arguments', - 'filter_expression', - 'current_approval_request', - 'approval', - 'is_approved' - ] - - expected_arguments = [ - 'defaults', - 'surveyId', - 'surveys' - ] - - expected_survey_fields = [ - 'message', - 'thanksMessage', - 'engagementButtonLabel', - 'title', - 'postAnswerUrl', - 'weight', - 'learnMoreUrl', - 'learnMoreMessage', - ] - - expected_default_fields = [ - 'message', - 'thanksMessage', - 'engagementButtonLabel', - 'postAnswerUrl', - 'learnMoreUrl', - 'learnMoreMessage' - ] - - # Verify we have at least one response - assert len(response) >= 1 - - # Take a look at the first response and look for the expected fields - record = response[0] - for field in expected_fields: - assert field in record - - # Do the arguments look right? - for argument in expected_arguments: - assert argument in record['arguments'] - - # Do the defaults look right? - for field in record['arguments']['defaults']: - assert field in expected_default_fields - - # Do survey fields look right? Look at the first one - for field in record['arguments']['surveys'][0]: - assert field in expected_survey_fields - - def filter_show_heartbeat(record): return record['name'] == 'show-heartbeat' diff --git a/docs/dev/api-tests.rst b/docs/dev/api-tests.rst new file mode 100644 index 00000000..87ef1b37 --- /dev/null +++ b/docs/dev/api-tests.rst @@ -0,0 +1,13 @@ +API Contract Tests +================== + +These tests are designed to look for changes to the recipe server API that are +not expected. + +To run these tests, use the following command from the root project directory. + +.. code-block:: bash + + py.test --env= contract-tests/ + +where ```` is one of `dev`, `stage`, or `prod` diff --git a/requirements.txt b/requirements.txt index ecd81308..50a9847b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -170,3 +170,5 @@ pyflakes==1.0.0 \ --hash=sha256:f39e33a4c03beead8774f005bd3ecf0c3f2f264fa0201de965fce0aff1d34263 flake8-junit-report==2.0.1 \ --hash=sha256:55914a99126d3b30cad2a042d95d466b5b368c7f69ef68ec7ec8321d3f0e2eee +configparser==3.5.0 \ + --hash=sha256:5308b47021bc2340965c371f0f058cc6971a04502638d4244225c49d80db273a diff --git a/test-requirements.txt b/test-requirements.txt deleted file mode 100644 index ebccf2e9..00000000 --- a/test-requirements.txt +++ /dev/null @@ -1,5 +0,0 @@ -configparser -flake8 -jsonschema -pytest -requests From b46a7d60a2d45ffdad577570aa61695eed94e4db Mon Sep 17 00:00:00 2001 From: Chris Hartjes Date: Tue, 12 Jul 2016 15:08:34 -0400 Subject: [PATCH 11/74] The --end flag should not be required for pytest runs --- conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 4be86535..a5533ce5 100644 --- a/conftest.py +++ b/conftest.py @@ -6,7 +6,7 @@ def pytest_addoption(parser): parser.addoption( "--env", action="store", - required=True, + required=False, help="Choose a test environment: dev, stage, or prod", ) From 51844a10a9a09fa24aeb8744d49cce13594d6990 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Tue, 12 Jul 2016 14:32:59 -0700 Subject: [PATCH 12/74] Document the basics of how to write filter expressions. fix bug 1284598 --- docs/dev/install.rst | 6 +- docs/index.rst | 25 +++- docs/user/filter_expressions.rst | 205 +++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 5 deletions(-) create mode 100644 docs/user/filter_expressions.rst diff --git a/docs/dev/install.rst b/docs/dev/install.rst index f548234e..5aab8777 100644 --- a/docs/dev/install.rst +++ b/docs/dev/install.rst @@ -109,11 +109,11 @@ Installation ./bin/download_geolite2.sh 10. Add some useful initial data to your database using the ``initial_data`` - command: + command: - .. code-block:: bash + .. code-block:: bash - python manage.py initial_data + python manage.py initial_data Once you've finished these steps, you should be able to start the site by running: diff --git a/docs/index.rst b/docs/index.rst index fbcd68d1..0a078af4 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -5,15 +5,36 @@ JavaScript to various clients based on certain rules. .. _SHIELD: https://wiki.mozilla.org/Firefox/SHIELD +User Documentation +------------------ +.. toctree:: + :maxdepth: 2 + + user/filter_expressions + +Developer Documentation +----------------------- .. toctree:: :maxdepth: 2 dev/install dev/workflow dev/concepts - qa/example dev/self-repair dev/driver dev/troubleshooting - qa/docker + +QA Documentation +---------------- +.. toctree:: + :maxdepth: 2 + + qa/example + qa/setup + +Operations Documentation +------------------------ +.. toctree:: + :maxdepth: 2 + ops/config diff --git a/docs/user/filter_expressions.rst b/docs/user/filter_expressions.rst new file mode 100644 index 00000000..d4a53148 --- /dev/null +++ b/docs/user/filter_expressions.rst @@ -0,0 +1,205 @@ +Filter Expressions +================== +Filter expressions describe which users a :ref:`recipe ` should be +executed for. They're executed locally in the client's browser and, if they +pass, the corresponding recipe is executed. Filter expressions have access to +information about the user, such as their location, locale, and Firefox version. + +Filter expressions are written using a language called JEXL_. JEXL is an +open-source expression language that is given a context (in this case, +information about the user's browser) and evaluates a statement using that +context. JEXL stands for "JavaScript Expression Language" and uses JavaScript +syntax for several (but not all) of its features. + +.. note:: The rest of this document includes examples of JEXL syntax that has + comments inline with the expressions. JEXL does **not** have any support for + comments in statements, but we're using them to make understanding our + examples easier. + +.. _JEXL: https://github.com/TechnologyAdvice/Jexl + +JEXL Basics +----------- +The `JEXL Readme`_ describes the syntax of the language in detail; the following +section covers the basics of writing valid JEXL expressions. + +.. note:: Normally, JEXL doesn't allow newlines or other whitespace besides + spaces in expressions, but filter expressions in Normandy allow arbitrary + whitespace. + +A JEXL expression evaluates down to a single value. JEXL supports several basic +types, such as numbers, strings (single or double quoted), and booleans. JEXL +also supports several operators for combining values, such as arithmetic, +boolean operators, comparisons, and string concatenation. + +.. code-block:: javascript + + // Arithmetic + 2 + 2 - 3 // == 1 + + // Numerical comparisons + 5 > 7 // == false + + // Boolean operators + false || 5 > 4 // == true + + // String concatenation + "Mozilla" + " " + "Firefox" // == "Mozilla Firefox" + +Expressions can be grouped using parenthesis: + +.. code-block:: javascript + + ((2 + 3) * 3) - 3 // == 7 + +JEXL also supports lists and objects (known as dictionaries in other languages) +as well as attribute access: + +.. code-block:: javascript + + [1, 2, 1].length // == 3 + {foo: 1, bar: 2}.foo // == 1 + +Unlike JavaScript, JEXL supports an ``in`` operator for checking if a substring +is in a string or if an element is in an array: + +.. code-block:: javascript + + "bar" in "foobarbaz" // == true + 3 in [1, 2, 3, 4] // == true + +The context passed to JEXL can be expressed using identifiers, which also +support attribute access: + +.. code-block:: javascript + + normandy.locale == 'en-US' // == true if the client's locale is en-US + +Another unique feature of JEXL is transforms, which modify the value given to +them. Transforms are applied to a value using the ``|`` operator, and may take +additional arguments passed in the expression: + +.. code-block:: javascript + + 'January 7th, 1980'|date // == a date object + +.. _JEXL Readme: https://github.com/TechnologyAdvice/Jexl#jexl--- + +Context +------- +This section defines the context passed to filter expressions when they are +evaluated. In other words, this is the client information available within +filter expressions. + +.. js:data:: normandy + + The ``normandy`` object contains general information about the client. + +.. js:attribute:: normandy.version + + **Example:** ``'47.0.1'`` + + String containing the user's Firefox version. + +.. js:attribute:: normandy.channel + + String containing the update channel. Valid values include, but are not + limited to: + + * ``'release'`` + * ``'aurora'`` + * ``'beta'`` + * ``'nightly'`` + * ``'default'`` (self-built or automated testing builds) + +.. js:attribute:: normandy.isDefaultBrowser + + Boolean specifying whether Firefox is set as the user's default browser. + +.. js:attribute:: normandy.searchEngine + + **Example:** ``'google'`` + + String containing the user's default search engine identifier. + +.. js:attribute:: normandy.syncSetup + + Boolean containing whether the user has set up Firefox Sync. + +.. js:attribute:: normandy.plugins + + An object mapping of plugin names to :js:class:`Plugin` objects describing + the plugins installed on the client. + +.. js:attribute:: normandy.locale + + **Example:** ``'en-US'`` + + String containing the user's locale. + +.. js:attribute:: normandy.country + + **Example:** ``'US'`` + + `ISO 3166-1 alpha-2`_ country code for the country that the user is located + in. This is determined via IP-based geolocation. + + .. _ISO 3166-1 alpha-2: https://en.wikipedia.org/wiki/ISO_3166-1 + +.. js:attribute:: normandy.request_time + + Date object set to the time and date that the user requested recipes from + Normandy. Useful for comparing against date ranges that a recipe is valid + for. + + .. code-block:: javascript + + // Do not run recipe after January 1st. + normandy.request_time < '2011-01-01'|date + +Transforms +---------- +This section describes the transforms available to filter expressions, and what +they do. They're documented as functions, and the first parameter to each +function is the value being transformed. + +.. js:function:: date(dateString) + + Parses a string as a date and returns a Date object. Strings should be in a + format recognized by `Date.parse()`_. + + :param string dateString: + String to parse as a date. + + .. code-block:: javascript + + '2011-10-10T14:48:00'|date // == Date object matching the given date + + .. _Date.parse(): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/parse + +Examples +-------- +This section lists some examples of commonly-used filter expressions. + +.. code-block:: javascript + + // Match users using the en-US locale while located in India + normandy.locale == 'en-US' && normandy.country == 'IN' + + // Match users in any English locale using Firefox Beta + ( + normandy.locale in ['en-US', 'en-AU', 'en-CA', 'en-GB', 'en-NZ', 'en-ZA'] + && normandy.channel == 'beta' + ) + + // Only run the recipe between January 1st, 2011 and January 7th, 2011 + ( + normandy.request_time > '2011-01-01T00:00:00+00:00'|date + && normandy.request_time < '2011-01-07T00:00:00+00:00'|date + ) + + // Match users located in the US who have Firefox as their default browser + normandy.country == 'US' && normandy.isDefaultBrowser + + // Match users with the Flash plugin installed + normandy.plugins['Shockwave Flash'] From 10e8bcb95194d16c2c302100294346725f480ce5 Mon Sep 17 00:00:00 2001 From: Mike Cooper Date: Tue, 12 Jul 2016 13:56:02 -0700 Subject: [PATCH 13/74] Standardize usage of jexl across the app Fixes bug 1280196 Fixes bug 1284597 --- karma.conf.js | 30 ++++++------ .../control/js/components/RecipeForm.js | 5 +- normandy/selfrepair/static/js/JexlEnv.js | 16 ++++++ .../static/js/self_repair_runner.js | 10 ++-- normandy/selfrepair/tests/test_JexlEnv.js | 49 +++++++++++++++++++ 5 files changed, 86 insertions(+), 24 deletions(-) create mode 100644 normandy/selfrepair/static/js/JexlEnv.js create mode 100644 normandy/selfrepair/tests/test_JexlEnv.js diff --git a/karma.conf.js b/karma.conf.js index 011646e9..7f9bf8bb 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -44,37 +44,37 @@ module.exports = function (config) { }, webpackServer: { - quiet: true, // Suppress all webpack messages, except errors + quiet: true, // Suppress *all* webpack messages, including errors }, - // test results reporter to use - // possible values: 'dots', 'progress' - // available reporters: https://npmjs.org/browse/keyword/karma-reporter + // test results reporter to use + // possible values: 'dots', 'progress' + // available reporters: https://npmjs.org/browse/keyword/karma-reporter reporters: ['spec'], - // web server port + // web server port port: 9876, - // enable / disable colors in the output (reporters and logs) + // enable / disable colors in the output (reporters and logs) colors: true, - // level of logging - // possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG + // level of logging + // possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG logLevel: config.LOG_INFO, - // enable / disable watching file and executing tests whenever any file changes + // enable / disable watching file and executing tests whenever any file changes autoWatch: true, - // start these browsers - // available browser launchers: https://npmjs.org/browse/keyword/karma-launcher + // start these browsers + // available browser launchers: https://npmjs.org/browse/keyword/karma-launcher browsers: ['Firefox'], - // Continuous Integration mode - // if true, Karma captures browsers, runs the tests and exits + // Continuous Integration mode + // if true, Karma captures browsers, runs the tests and exits singleRun: false, - // Concurrency level - // how many browser should be started simultaneous + // Concurrency level + // how many browser should be started simultaneous concurrency: Infinity, }; diff --git a/normandy/control/static/control/js/components/RecipeForm.js b/normandy/control/static/control/js/components/RecipeForm.js index c8cc79cc..9658b641 100644 --- a/normandy/control/static/control/js/components/RecipeForm.js +++ b/normandy/control/static/control/js/components/RecipeForm.js @@ -2,7 +2,6 @@ import React, { PropTypes as pt } from 'react'; import { Link } from 'react-router'; import { push } from 'react-router-redux'; import { destroy, reduxForm, getValues } from 'redux-form'; -import jexl from 'jexl'; import { makeApiRequest, recipeUpdated, recipeAdded, showNotification } from '../actions/ControlActions.js'; @@ -10,6 +9,7 @@ import composeRecipeContainer from './RecipeContainer.js'; import ActionForm from './ActionForm.js'; import CheckboxField from './form_fields/CheckboxField.js'; import FormField from './form_fields/FormFieldWrapper.js'; +import JexlEnv from '../../../../../selfrepair/static/js/JexlEnv.js'; export class RecipeForm extends React.Component { propTypes = { @@ -53,7 +53,8 @@ export class RecipeForm extends React.Component { validateForm(formValues) { const jexlExpression = formValues.filter_expression; - return jexl.eval(jexlExpression, {}); + const jexlEnv = new JexlEnv({}); + return jexlEnv.eval(jexlExpression, {}); } submitForm() { diff --git a/normandy/selfrepair/static/js/JexlEnv.js b/normandy/selfrepair/static/js/JexlEnv.js new file mode 100644 index 00000000..a42f1d79 --- /dev/null +++ b/normandy/selfrepair/static/js/JexlEnv.js @@ -0,0 +1,16 @@ +import { Jexl } from 'jexl'; +import { stableSample } from './utils.js'; + +export default class JexlEnv { + constructor(context) { + this.context = context; + this.jexl = new Jexl(); + this.jexl.addTransform('date', value => new Date(value)); + this.jexl.addTransform('stableSample', stableSample); + } + + eval(expr) { + const oneLineExpr = expr.replace('\n', ' '); + return this.jexl.eval(oneLineExpr, this.context); + } +} diff --git a/normandy/selfrepair/static/js/self_repair_runner.js b/normandy/selfrepair/static/js/self_repair_runner.js index 7c6c5099..04a52e08 100644 --- a/normandy/selfrepair/static/js/self_repair_runner.js +++ b/normandy/selfrepair/static/js/self_repair_runner.js @@ -1,9 +1,6 @@ import uuid from 'node-uuid'; -import jexl from 'jexl'; - - -jexl.addTransform('date', value => new Date(value)); +import JexlEnv from './JexlEnv.js'; const registeredActions = {}; window.registerAction = (name, ActionClass) => { @@ -134,8 +131,7 @@ export function filterContext(driver) { */ export function doesRecipeMatch(recipe, context) { // Remove newlines, which are invalid in JEXL + const jexlEnv = new JexlEnv(context); const filterExpression = recipe.filterExpression.replace(/\r?\n|\r/g, ''); - - return jexl.eval(filterExpression, context) - .then(value => [recipe, !!value]); + return jexlEnv.eval(filterExpression).then(value => [recipe, !!value]); } diff --git a/normandy/selfrepair/tests/test_JexlEnv.js b/normandy/selfrepair/tests/test_JexlEnv.js new file mode 100644 index 00000000..f8f7e8ef --- /dev/null +++ b/normandy/selfrepair/tests/test_JexlEnv.js @@ -0,0 +1,49 @@ +import JexlEnv from '../static/js/JexlEnv.js'; + +describe('JexlEnv', () => { + let jexlEnv; + + beforeAll(() => { + jexlEnv = new JexlEnv(); + }); + + it('should pull values from the context', () => { + const marker = Symbol(); + const context = { data: { marker } }; + jexlEnv = new JexlEnv(context); + return jexlEnv.eval('data.marker') + .then(val => expect(val).toEqual(marker)); + }); + + it('should execute simple expressions', () => ( + jexlEnv.eval('2+2') + .then(val => expect(val).toEqual(4)) + )); + + it('should execute multiline statements', () => ( + jexlEnv.eval('1 + 5 *\n8 + 1') + .then(val => expect(val).toEqual(42)) + )); + + it('should have a date filter', () => ( + jexlEnv.eval('"2016-07-12T00:00:00"|date') + .then(val => expect(val).toEqual(new Date(2016, 6, 12))) + )); + + describe('stable sample filter', () => { + it('should have a stableSample filter', () => ( + // Expect to not fail + jexlEnv.eval('"test"|stableSample(0.5)') + )); + + it('should return true for matching samples', () => ( + jexlEnv.eval('"test"|stableSample(1.0)') + .then(val => expect(val).toEqual(true)) + )); + + it('should return false for matching samples', () => ( + jexlEnv.eval('"test"|stableSample(0.0)') + .then(val => expect(val).toEqual(false)) + )); + }); +}); From 71394a21e3e34c3986e930eda825b97d2b23b6e9 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Wed, 13 Jul 2016 10:00:01 -0700 Subject: [PATCH 14/74] Review feedback. --- docs/user/filter_expressions.rst | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/user/filter_expressions.rst b/docs/user/filter_expressions.rst index d4a53148..b01f8201 100644 --- a/docs/user/filter_expressions.rst +++ b/docs/user/filter_expressions.rst @@ -81,7 +81,7 @@ additional arguments passed in the expression: .. code-block:: javascript - 'January 7th, 1980'|date // == a date object + '1980-01-07'|date // == a date object .. _JEXL Readme: https://github.com/TechnologyAdvice/Jexl#jexl--- @@ -165,8 +165,8 @@ function is the value being transformed. .. js:function:: date(dateString) - Parses a string as a date and returns a Date object. Strings should be in a - format recognized by `Date.parse()`_. + Parses a string as a date and returns a Date object. Date strings should be + in `ISO 8601`_ format. :param string dateString: String to parse as a date. @@ -175,7 +175,7 @@ function is the value being transformed. '2011-10-10T14:48:00'|date // == Date object matching the given date - .. _Date.parse(): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/parse + .. _ISO 8601: https://www.w3.org/TR/NOTE-datetime Examples -------- @@ -201,5 +201,7 @@ This section lists some examples of commonly-used filter expressions. // Match users located in the US who have Firefox as their default browser normandy.country == 'US' && normandy.isDefaultBrowser - // Match users with the Flash plugin installed + // Match users with the Flash plugin installed. If Flash is missing, the + // plugin list returns `undefined`, which is a falsy value in JavaScript and + // fails the match. Otherwise, it returns a plugin object, which is truthy. normandy.plugins['Shockwave Flash'] From e325ebbe02a01da97b7c3037fcd46437bf63b7b3 Mon Sep 17 00:00:00 2001 From: Chris Hartjes Date: Thu, 14 Jul 2016 09:43:39 -0400 Subject: [PATCH 15/74] Added a default value for --env so as to not break CI builds --- conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conftest.py b/conftest.py index a5533ce5..ea91cc78 100644 --- a/conftest.py +++ b/conftest.py @@ -7,6 +7,7 @@ def pytest_addoption(parser): "--env", action="store", required=False, + default="dev", help="Choose a test environment: dev, stage, or prod", ) From f92a4dbaad17aac0e7436f11f659b847aba54fe6 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Thu, 14 Jul 2016 10:13:26 -0700 Subject: [PATCH 16/74] Add stableSample to JEXL docs. --- docs/user/filter_expressions.rst | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/user/filter_expressions.rst b/docs/user/filter_expressions.rst index b01f8201..d2e9063a 100644 --- a/docs/user/filter_expressions.rst +++ b/docs/user/filter_expressions.rst @@ -163,6 +163,32 @@ This section describes the transforms available to filter expressions, and what they do. They're documented as functions, and the first parameter to each function is the value being transformed. +.. js:function:: stableSample(input, rate) + + Randomly returns ``true`` or ``false`` based on the given sample rate. Used + to sample over the set of matched users. + + Sampling with this transform is stable over the input, meaning that the same + input and sample rate will always result in the same return value. The most + common use is to pass in a unique user ID and a recipe ID as the input; this + means that each user will consistently run or not run a recipe. + + Without stable sampling, a user might execute a recipe on Monday, and then + not execute it on Tuesday. In addition, without stable sampling, a recipe + would be seen by a different percentage of users each day, and over time this + would add up such that the recipe is seen by more than the percent sampled. + + :param input: + A value for the sample to be stable over. + :param number rate: + A number between ``0`` and ``1`` with the sample rate. For example, + ``0.5`` would be a 50% sample rate. + + .. code-block:: javascript + + // True 50% of the time, stable per-user per-recipe. + [normandy.userId, normandy.recipe.id]|stableSample(0.5) + .. js:function:: date(dateString) Parses a string as a date and returns a Date object. Date strings should be @@ -186,6 +212,12 @@ This section lists some examples of commonly-used filter expressions. // Match users using the en-US locale while located in India normandy.locale == 'en-US' && normandy.country == 'IN' + // Match 10% of users in the fr locale. + ( + normandy.locale == 'fr' + && [normandy.userId, normandy.recipe.id]|stableSample(0.1) + ) + // Match users in any English locale using Firefox Beta ( normandy.locale in ['en-US', 'en-AU', 'en-CA', 'en-GB', 'en-NZ', 'en-ZA'] From 3f538c2dbbdf4e1c023d174c4ff21b32dc0c78a0 Mon Sep 17 00:00:00 2001 From: Mike Cooper Date: Thu, 14 Jul 2016 11:05:58 -0700 Subject: [PATCH 17/74] Add docker-image-shasum256.txt to CI artifacts Fixes bug 1273332 --- circle.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/circle.yml b/circle.yml index 9d15cd7e..7902be0c 100644 --- a/circle.yml +++ b/circle.yml @@ -13,6 +13,8 @@ dependencies: - if [ -e ~/cache/docker/image.tar ]; then echo "Loading image.tar"; docker load -i ~/cache/docker/image.tar || rm ~/cache/docker/image.tar; fi # build image - docker build -t normandy:build . + # write the sha256 sum to an artifact to make image verification easier + - docker inspect -f '{{.Config.Image}}' normandy:build | tee $CIRCLE_ARTIFACTS/docker-image-shasum256.txt # Get MaxMind GeoIP database - cd ~/cache/ && ~/normandy/bin/download_geolite2.sh From 4666f70e6cc4ce187e1243fa834265b21aa18216 Mon Sep 17 00:00:00 2001 From: Mike Cooper Date: Thu, 14 Jul 2016 16:54:15 -0700 Subject: [PATCH 18/74] Feedback --- .../control/static/control/js/components/RecipeForm.js | 4 ++-- .../static/js/{JexlEnv.js => JexlEnvironment.js} | 4 ++-- normandy/selfrepair/static/js/self_repair_runner.js | 7 +++---- normandy/selfrepair/tests/test_JexlEnv.js | 8 ++++---- 4 files changed, 11 insertions(+), 12 deletions(-) rename normandy/selfrepair/static/js/{JexlEnv.js => JexlEnvironment.js} (78%) diff --git a/normandy/control/static/control/js/components/RecipeForm.js b/normandy/control/static/control/js/components/RecipeForm.js index 9658b641..a90c1bab 100644 --- a/normandy/control/static/control/js/components/RecipeForm.js +++ b/normandy/control/static/control/js/components/RecipeForm.js @@ -9,7 +9,7 @@ import composeRecipeContainer from './RecipeContainer.js'; import ActionForm from './ActionForm.js'; import CheckboxField from './form_fields/CheckboxField.js'; import FormField from './form_fields/FormFieldWrapper.js'; -import JexlEnv from '../../../../../selfrepair/static/js/JexlEnv.js'; +import JexlEnvironment from '../../../../../selfrepair/static/js/JexlEnvironment.js'; export class RecipeForm extends React.Component { propTypes = { @@ -53,7 +53,7 @@ export class RecipeForm extends React.Component { validateForm(formValues) { const jexlExpression = formValues.filter_expression; - const jexlEnv = new JexlEnv({}); + const jexlEnv = new JexlEnvironment({}); return jexlEnv.eval(jexlExpression, {}); } diff --git a/normandy/selfrepair/static/js/JexlEnv.js b/normandy/selfrepair/static/js/JexlEnvironment.js similarity index 78% rename from normandy/selfrepair/static/js/JexlEnv.js rename to normandy/selfrepair/static/js/JexlEnvironment.js index a42f1d79..450ecfe2 100644 --- a/normandy/selfrepair/static/js/JexlEnv.js +++ b/normandy/selfrepair/static/js/JexlEnvironment.js @@ -1,7 +1,7 @@ import { Jexl } from 'jexl'; import { stableSample } from './utils.js'; -export default class JexlEnv { +export default class JexlEnvironment { constructor(context) { this.context = context; this.jexl = new Jexl(); @@ -10,7 +10,7 @@ export default class JexlEnv { } eval(expr) { - const oneLineExpr = expr.replace('\n', ' '); + const oneLineExpr = expr.replace(/\r?\n|\r/g, ' '); return this.jexl.eval(oneLineExpr, this.context); } } diff --git a/normandy/selfrepair/static/js/self_repair_runner.js b/normandy/selfrepair/static/js/self_repair_runner.js index 04a52e08..cccd9ddd 100644 --- a/normandy/selfrepair/static/js/self_repair_runner.js +++ b/normandy/selfrepair/static/js/self_repair_runner.js @@ -1,6 +1,6 @@ import uuid from 'node-uuid'; -import JexlEnv from './JexlEnv.js'; +import JexlEnvironment from './JexlEnvironment.js'; const registeredActions = {}; window.registerAction = (name, ActionClass) => { @@ -131,7 +131,6 @@ export function filterContext(driver) { */ export function doesRecipeMatch(recipe, context) { // Remove newlines, which are invalid in JEXL - const jexlEnv = new JexlEnv(context); - const filterExpression = recipe.filterExpression.replace(/\r?\n|\r/g, ''); - return jexlEnv.eval(filterExpression).then(value => [recipe, !!value]); + const jexlEnv = new JexlEnvironment(context); + return jexlEnv.eval(recipe.filterExpression).then(value => [recipe, !!value]); } diff --git a/normandy/selfrepair/tests/test_JexlEnv.js b/normandy/selfrepair/tests/test_JexlEnv.js index f8f7e8ef..f9610e50 100644 --- a/normandy/selfrepair/tests/test_JexlEnv.js +++ b/normandy/selfrepair/tests/test_JexlEnv.js @@ -1,16 +1,16 @@ -import JexlEnv from '../static/js/JexlEnv.js'; +import JexlEnvironment from '../static/js/JexlEnvironment.js'; -describe('JexlEnv', () => { +describe('JexlEnvironment', () => { let jexlEnv; beforeAll(() => { - jexlEnv = new JexlEnv(); + jexlEnv = new JexlEnvironment(); }); it('should pull values from the context', () => { const marker = Symbol(); const context = { data: { marker } }; - jexlEnv = new JexlEnv(context); + jexlEnv = new JexlEnvironment(context); return jexlEnv.eval('data.marker') .then(val => expect(val).toEqual(marker)); }); From d5fd673bfc63503607fee2700c91d51d0f21e9d4 Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Wed, 13 Jul 2016 07:53:29 -0400 Subject: [PATCH 19/74] Validate argument errors server-side --- .../control/js/components/RecipeForm.js | 2 + normandy/recipes/api/serializers.py | 38 +++++++++++++++++++ normandy/recipes/validators.py | 25 ++++++++++++ 3 files changed, 65 insertions(+) diff --git a/normandy/control/static/control/js/components/RecipeForm.js b/normandy/control/static/control/js/components/RecipeForm.js index a90c1bab..0c762e39 100644 --- a/normandy/control/static/control/js/components/RecipeForm.js +++ b/normandy/control/static/control/js/components/RecipeForm.js @@ -2,6 +2,7 @@ import React, { PropTypes as pt } from 'react'; import { Link } from 'react-router'; import { push } from 'react-router-redux'; import { destroy, reduxForm, getValues } from 'redux-form'; +import { _ } from 'underscore'; import { makeApiRequest, recipeUpdated, recipeAdded, showNotification } from '../actions/ControlActions.js'; @@ -63,6 +64,7 @@ export class RecipeForm extends React.Component { const recipeFormValues = getValues(formState.recipe); const actionFormValues = getValues(formState.action); const combinedFormValues = { ...recipeFormValues, arguments: actionFormValues }; + const requestBody = { recipe: combinedFormValues, recipeId }; return this.validateForm(combinedFormValues) diff --git a/normandy/recipes/api/serializers.py b/normandy/recipes/api/serializers.py index ea4e31a4..b2654016 100644 --- a/normandy/recipes/api/serializers.py +++ b/normandy/recipes/api/serializers.py @@ -7,6 +7,7 @@ from normandy.base.api.serializers import UserSerializer from normandy.recipes.api.fields import ActionImplementationHyperlinkField from normandy.recipes.models import (Action, Recipe, Approval, ApprovalRequest, ApprovalRequestComment) +from normandy.recipes.validators import Validator class ActionSerializer(serializers.ModelSerializer): @@ -102,6 +103,43 @@ class RecipeSerializer(serializers.ModelSerializer): 'is_approved', ] + def validate_arguments(self, value): + # Get the schema associated with the selected action + try: + schema = Action.objects.get(name=self.initial_data.get('action')).arguments_schema + except: + raise serializers.ValidationError('Could not find arguments schema.') + + schemaValidator = Validator(schema) + errorResponse = {} + errors = sorted(schemaValidator.iter_errors(value), key=lambda e: e.path) + + for error in errors: + currentLevel = errorResponse + + # Loop through the path of the current error + # e.g. ['surveys'][0]['weight'] + for index, path in enumerate(error.path): + # If this key already exists in our error response, step into it + if path in currentLevel: + currentLevel = currentLevel[path] + continue + else: + # If we haven't reached the end of the path, add this path + # as a key in our error response object and step into it + if index < len(error.path) - 1: + currentLevel[path] = {} + currentLevel = currentLevel[path] + continue + # If we've reached the final path, set the error message + else: + currentLevel[path] = error.message + + if (errorResponse): + raise serializers.ValidationError(errorResponse) + + return value + class ClientSerializer(serializers.Serializer): country = serializers.CharField() diff --git a/normandy/recipes/validators.py b/normandy/recipes/validators.py index 7dc3ef41..52a7f07b 100644 --- a/normandy/recipes/validators.py +++ b/normandy/recipes/validators.py @@ -1,8 +1,33 @@ import json +import jsonschema from django.core.exceptions import ValidationError +# Add path to required validator so we can get property name +def _required(validator, required, instance, schema): + '''Validate 'required' properties.''' + if not validator.is_type(instance, 'object'): + return + + for index, requirement in enumerate(required): + if requirement not in instance: + error = jsonschema.ValidationError( + 'This field may not be blank.', + path=[requirement] + ) + yield error + + +# Construct validator as extension of Json Schema Draft 4. +Validator = jsonschema.validators.extend( + validator=jsonschema.validators.Draft4Validator, + validators={ + 'required': _required + } +) + + def validate_json(value): """ Validate that a given value can be successfully parsed as JSON. From 6525129fcbab42144e42842976447c5304ab0d51 Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Wed, 13 Jul 2016 08:30:40 -0400 Subject: [PATCH 20/74] Display action form errors --- .../control/js/components/RecipeForm.js | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/normandy/control/static/control/js/components/RecipeForm.js b/normandy/control/static/control/js/components/RecipeForm.js index 0c762e39..6af3ae76 100644 --- a/normandy/control/static/control/js/components/RecipeForm.js +++ b/normandy/control/static/control/js/components/RecipeForm.js @@ -1,7 +1,7 @@ import React, { PropTypes as pt } from 'react'; import { Link } from 'react-router'; import { push } from 'react-router-redux'; -import { destroy, reduxForm, getValues } from 'redux-form'; +import { destroy, stopSubmit, reduxForm, getValues } from 'redux-form'; import { _ } from 'underscore'; import { makeApiRequest, recipeUpdated, recipeAdded, showNotification } @@ -64,7 +64,6 @@ export class RecipeForm extends React.Component { const recipeFormValues = getValues(formState.recipe); const actionFormValues = getValues(formState.action); const combinedFormValues = { ...recipeFormValues, arguments: actionFormValues }; - const requestBody = { recipe: combinedFormValues, recipeId }; return this.validateForm(combinedFormValues) @@ -155,10 +154,37 @@ export default composeRecipeContainer(reduxForm({ ? props.location.state.selectedRevision : null; + const formatErrors = payload => { + let errors = payload; + + if (_.isObject(payload)) { + const invalidFields = Object.keys(payload); + if (invalidFields.length > 0) { + errors = isNaN(invalidFields[0]) ? {} : []; + + invalidFields.forEach(fieldName => { + errors[fieldName] = formatErrors(payload[fieldName]); + }); + } + } + + return errors; + }; + + const onSubmitFail = errors => { + const { dispatch } = props; + const actionFormErrors = errors['arguments']; // eslint-disable-line dot-notation + + if (actionFormErrors) { + dispatch(stopSubmit('action', formatErrors(actionFormErrors))); + } + }; + return { fields, initialValues: selectedRecipeRevision || props.recipe, viewingRevision: selectedRecipeRevision || props.location.query.revisionId, formState: state.form, + onSubmitFail, }; })(RecipeForm)); From 2d05ede4db8bf80834e04ffb5f9d0ec11982851d Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Wed, 13 Jul 2016 14:23:02 -0400 Subject: [PATCH 21/74] Check for empty strings in required validator --- normandy/recipes/validators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/normandy/recipes/validators.py b/normandy/recipes/validators.py index 52a7f07b..76d7dae7 100644 --- a/normandy/recipes/validators.py +++ b/normandy/recipes/validators.py @@ -11,7 +11,7 @@ def _required(validator, required, instance, schema): return for index, requirement in enumerate(required): - if requirement not in instance: + if requirement not in instance or instance[requirement] == '': error = jsonschema.ValidationError( 'This field may not be blank.', path=[requirement] From 7cea45418b6216dc6dc6e97afbc52c6e0eafb166 Mon Sep 17 00:00:00 2001 From: Brittany Storoz Date: Thu, 14 Jul 2016 11:33:58 -0400 Subject: [PATCH 22/74] Normalize weight field from string to integer --- .../components/action_forms/HeartbeatForm.js | 28 ++++++++----- .../form_fields/FormFieldWrapper.js | 7 ++++ .../js/components/form_fields/NumberField.js | 42 +++++++++++++++++++ 3 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 normandy/control/static/control/js/components/form_fields/NumberField.js diff --git a/normandy/control/static/control/js/components/action_forms/HeartbeatForm.js b/normandy/control/static/control/js/components/action_forms/HeartbeatForm.js index 485d9b86..67bd7ef2 100644 --- a/normandy/control/static/control/js/components/action_forms/HeartbeatForm.js +++ b/normandy/control/static/control/js/components/action_forms/HeartbeatForm.js @@ -21,8 +21,6 @@ export const HeartbeatFormFields = [ 'surveys[].weight', ]; -const formatLabel = labelName => labelName.replace(/([A-Z])/g, ' $1').toLowerCase(); - const SurveyListItem = props => { const { survey, surveyIndex, isSelected, deleteSurvey, onClick } = props; return ( @@ -51,9 +49,11 @@ const SurveyForm = props => { const { selectedSurvey, fields, showDefaults } = props; const surveyObject = selectedSurvey || fields.defaults; let headerText = 'Default Survey Values'; + let showAdditionalSurveyFields = false; let containerClasses = classNames('fluid-8', { active: selectedSurvey }); if (selectedSurvey) { + showAdditionalSurveyFields = true; headerText = selectedSurvey.title.initialValue || 'New survey'; } @@ -65,16 +65,22 @@ const SurveyForm = props => { }

{headerText}

- { - Object.keys(surveyObject).map(fieldName => - - ) + + {showAdditionalSurveyFields && + } + + + + + + + + + {showAdditionalSurveyFields && + + } + ); }; diff --git a/normandy/control/static/control/js/components/form_fields/FormFieldWrapper.js b/normandy/control/static/control/js/components/form_fields/FormFieldWrapper.js index 0440ba05..d4ee08de 100644 --- a/normandy/control/static/control/js/components/form_fields/FormFieldWrapper.js +++ b/normandy/control/static/control/js/components/form_fields/FormFieldWrapper.js @@ -1,4 +1,5 @@ import React, { PropTypes as pt } from 'react'; +import NumberField from './NumberField.js'; const SelectMenu = props => { const { options, onChange, field } = props; @@ -26,6 +27,9 @@ const FormField = props => { case 'text': fieldType = (); break; + case 'number': + fieldType = (); + break; case 'textarea': fieldType = (