镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content
This repository was archived by the owner on Dec 7, 2022. It is now read-only.

SELA-1352 - Allow Launch Darkly query parameter overrides - #6

Merged
jacobmoretti merged 12 commits into
masterfrom
story/SELA-1352
Dec 30, 2016
Merged

jacobmoretti merged 12 commits into
masterfrom
story/SELA-1352

Conversation

@sethbattin

@sethbattin sethbattin commented Dec 21, 2016 •

Copy link
Copy Markdown
Contributor

This change checks window.location for query parameters to override LD service data.

Usage

Enabling Boolean Feature Flags

You can enable a set of boolean feature flags with a comma-delimited list in the features query parameter:

POST /users?features=send-onboarding-email
# Overrides the `send-onboarding-email` boolean feature flag, setting it to `true`
GET /users/101?features=show-user-email,user-nicknames,hide-inactive-users
# Enables the `show-user-email`, `user-nicknames`, and `hide-inactive-users` feature flags

Advanced Feature Flag Overriding

If you need to temporarily set a boolean feature flag to false or override the variation reported by a multivariate feature flag, you can use features.{feature_flag} query parameters:

POST /users?features.verify-email=false&features.email-frequency=weekly
# Disables the `verify-email` feature flag and sets the `email-frequency` variation to "weekly"

The values "true" and "false" are converted into true and false boolean values. If the query parameter value is omitted, then the feature flag will be reported as enabled:

GET /users/101?features.show-user-email
# Enables the `show-user-email` feature flag

Missing features

  • Changes are not "sticky"; query param overrides must be included in every request.
  • Edits to this component do not propagate through hot module reloading; gluestick restart is required
  • Query parameters are not passed to backend calls [followup issue here]
  • New code test coverage

Example

This is how I tested the feature:

// /src/components/Home.js
/* @flow */
import React, { Component } from "react";
import { FeatureFlag } from "react-launch-darkly";

export default class Home extends Component {
  render () {
    return (
      <div>
        <div>Home</div>
        <FeatureFlag
          flagKey="test-key"
          renderFeatureCallback={this._renderFeature}
          renderDefaultCallback={this._renderDefault}
        />
      </div>
    );
  }
  _renderFeature () {
    return (
      <div>I'm a new feature1111</div>
    );
  }
  _renderDefault() {
    return (
      <div>I'm NOT a new feature223</div>
    )
  }
}

// /src/components/MasterLayout.js
/* @flow */
import React, { Component, PropTypes } from "react";
import Helmet from "react-helmet";
import config from "../config/application";
import { LaunchDarkly } from "react-launch-darkly";


export default class MasterLayout extends Component {
  static propTypes = {
    children: PropTypes.any
  };

  render () {
    return (
      <div>
        <Helmet {...config.head}/>
        <LaunchDarkly {...config.launchDarkly} >
          {this.props.children}
        </LaunchDarkly>
      </div>
    );
  }
}

// /src/config/application.js
// ...
const config = {
  development: {
// ....
    launchDarkly: {
      clientId: "local",
      user: {
        key: "myuserkey"
      }
    }
  },
//....

@jacobmoretti jacobmoretti left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good so far! Just a few comments. I think its missing a piece of functionality. Also, whats the current state of the tests? I saw comments left in so wasn't sure.

Comment thread src/lib/launchDarkly.js Outdated
let override;
/**
* Follow this overriding convention:
* @link https://git.corp.tc/capsela/tc_feature_flagging#overriding-feature-flags

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This link should be removed and include general guidance on how the query params should be formatted. Since its a link to our enterprise github, its not available for the public and doesn't provide any value for anyone but us.

Comment thread src/lib/launchDarkly.js
* @link https://git.corp.tc/capsela/tc_feature_flagging#overriding-feature-flags
**/
const query = url.parse(window.location.toString(), true).query;
const queryFlag = query["features." + flagKey];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This works for supporting the advanced overriding, e.g., ?features.flagA=true but I'm not seeing support for the basic list of boolean flags, e.g., ?features=flagA,flagB,flagC.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right and I will fix that.

@sethbattin sethbattin changed the title WIP: SELA-1352 - Allow Launch Darkly query parameter overrides SELA-1352 - Allow Launch Darkly query parameter overrides Dec 27, 2016
@sethbattin

Copy link
Copy Markdown
Contributor Author

@jacobmoretti I added the basic override cases, and I inlined the spec

afterEach(() => {
_sandbox.restore();
})
it("param \"features.flag=false\" overrides LD data \"on\"", () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor nitpick but instead of escaping the double quotes here, lets just use single quotes or backticks.

@jacobmoretti

Copy link
Copy Markdown
Collaborator

@sethbattin Ok, ignore my previous comment. I just added travis-ci. So far changes look good to me, there are just lint issues right now: yarn lint.

@sethbattin

Copy link
Copy Markdown
Contributor Author

@jacobmoretti looks like I don't have write access, so I can't merge. If you're still good with it, would you check it off for me?

@jacobmoretti
jacobmoretti merged commit 897343c into master Dec 30, 2016
@jacobmoretti
jacobmoretti deleted the story/SELA-1352 branch December 30, 2016 18:16
@jacobmoretti

Copy link
Copy Markdown
Collaborator

@sethbattin Thanks, merged!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants