Skip to content

feat(AudienceEvaluator): Add the ability to provide custom condition evaluators#288

Merged
mikeproeng37 merged 13 commits into
masterfrom
james/custom_condition_evaluators_OG
Jun 19, 2019
Merged

feat(AudienceEvaluator): Add the ability to provide custom condition evaluators#288
mikeproeng37 merged 13 commits into
masterfrom
james/custom_condition_evaluators_OG

Conversation

@jamesopti
Copy link
Copy Markdown
Contributor

@jamesopti jamesopti commented Jun 7, 2019

Summary

This PR (replacing #216) adds the ability for an SDK consumer to pass their own set of condition evaluators when creating an optimizely instance.

The core motivator for this is to be able to evaluate WEB audience conditions using the Javascript SDK as part of the EDGE project (go/edge).

Future fullstack use cases for this functionality might include audience condition plugins, where the customer would define their own match types and provide their own matchers.

/**
 * Condition evaluators for conditions that can be evaluated in a Cloudflare worker using
 * information like cookies, query parameters, userAgent, etc...
 */
function getConditionEvaluators(inputs) {
  return {
    cookies: {
      evaluate: matchCookies.match.bind(this, { cookies: inputs.cookies }),
    },
    device: {
      evaluate: matchDevice.match.bind(this, { device: Detect.parseUA(inputs.ua).device }),
    },
    ...
    /* Other custom condition evaluators */
  }
}

var optlyInstance = optimizely.createInstance({
  datafile: datafile,
  conditionEvaluators: getConditionEvaluators(inputs),
  skipJSONValidation: true
});

Test plan

Fixed existing tests. May need to add more unit tests.

Issues

https://optimizely.atlassian.net/browse/CJS-3034

rebase with latest audience editor changes

updates per code review
@jamesopti jamesopti force-pushed the james/custom_condition_evaluators_OG branch from 0112ff9 to 2ff9d2e Compare June 7, 2019 21:11
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 7, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.531% when pulling 4da5d4a on james/custom_condition_evaluators_OG into bd49de0 on master.

if (condition.type !== CUSTOM_ATTRIBUTE_CONDITION_TYPE) {
logger.log(LOG_LEVEL.WARNING, sprintf(LOG_MESSAGES.UNKNOWN_CONDITION_TYPE, MODULE_NAME, JSON.stringify(condition)));
return null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

delete? because we're handling this in typeToEvaluate - we only call this if there's a type.

@lpappone lpappone changed the title James/custom condition evaluators og feat(AudienceEvaluator): Add the ability to provide custom condition evaluators Jun 7, 2019
* @constructor
*/
function AudienceEvaluator(logger, UNSTABLE_conditionEvaluators) {
this.logger = logger;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of passing logger, we can import a singleton logger and use it directly (example from project_config_manager)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

Mostly looks good. I just have a question.

Comment thread packages/optimizely-sdk/lib/core/audience_evaluator/index.js
* @constructor
*/
function AudienceEvaluator(logger, UNSTABLE_conditionEvaluators) {
this.logger = logger;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Comment thread packages/optimizely-sdk/lib/core/audience_evaluator/index.js
Comment thread packages/optimizely-sdk/lib/core/audience_evaluator/index.js
Copy link
Copy Markdown
Contributor

@nchilada nchilada left a comment