Feature flag suggested changes

.env

@@ -29,3 +29,4 @@ SPLIT_PASSWORD=split_password
 SPLIT_USERNAME=split_username
 STRIPE_API_KEY=stripe_api_key
 STRIPE_PUBLISHABLE_KEY=stripe_publishable_key
+FEATURES_ADMIN_PASSWORD=supersecret

Gemfile

@@ -10,6 +10,9 @@ gem "autoprefixer-rails"
 gem "bourbon", "~> 5.0"
 gem "email_validator"
 gem "faraday"
+gem "flipper"
+gem "flipper-active_record"
+gem "flipper-ui"
 gem "haml-rails", "~> 2.0"
 gem "high_voltage"
 gem "inifile"

Gemfile.lock

@@ -122,6 +122,15 @@ GEM
  faraday (1.0.0)
  multipart-post (>= 1.2, < 3)
  ffi (1.12.2)
+ flipper (0.17.2)
+ flipper-active_record (0.17.2)
+ activerecord (>= 4.2, < 7)
+ flipper (~> 0.17.2)
+ flipper-ui (0.17.2)
+ erubi (>= 1.0.0, < 2.0.0)
+ flipper (~> 0.17.2)
+ rack (>= 1.4, < 3)
+ rack-protection (>= 1.5.3, < 2.1.0)
  foreman (0.87.0)
  globalid (0.4.2)
  activesupport (>= 4.2.0)
@@ -383,6 +392,9 @@ DEPENDENCIES
  email_validator
  factory_girl_rails
  faraday
+ flipper
+ flipper-active_record
+ flipper-ui
  foreman
  haml-rails (~> 2.0)
  high_voltage

app/models/owner.rb

@@ -64,6 +64,10 @@ def metered_plan?
  whitelisted?
  end
 
+ def flipper_id
+ "Owner:#{id}"
+ end
+
  private
 
  def hound_config

app/models/violation.rb

@@ -23,6 +23,10 @@ def source
  crypt.decrypt_and_verify(self[:source]) if self[:source]
  end
 
+ def owner
+ file_review.build.repo.owner
+ end
+
  private
 
  def crypt

app/services/suggest_changes.rb

@@ -7,17 +7,21 @@ def initialize(violation)
  end
 
  def call
- violation.messages.each do |message|
- apply_suggestion(message)
- break if suggestion.present?
- end
+ if Flipper[:suggested_changes].enabled?(violation.owner)
+ violation.messages.each do |message|
+ apply_suggestion(message)
+ break if suggestion.present?
+ end
 
- messages = violation.messages.join(CommentingPolicy::COMMENT_LINE_DELIMITER)
+  formatted_messages = violation.messages.join(CommentingPolicy::COMMENT_LINE_DELIMITER)
 
- if suggestion.blank?
- messages
+ if suggestion.blank?
+ formatted_messages
+ else
+ formatted_messages + "<br>\n```suggestion\n#{suggestion}\n```"
+ end
  else
- messages + "<br>\n```suggestion\n#{suggestion}\n```"
+ violation.messages.join(CommentingPolicy::COMMENT_LINE_DELIMITER)
  end
  end
 

config/initializers/flipper.rb

@@ -0,0 +1,8 @@
+require "flipper"
+require "flipper/adapters/active_record"
+
+Flipper.configure do |config|
+ config.default do
+ Flipper.new(Flipper::Adapters::ActiveRecord.new)
+ end
+end

config/routes.rb

@@ -20,6 +20,16 @@
  mount Sidekiq::Web, at: "/jobs"
  mount Split::Dashboard, at: "/split"
 
+ flipper_app = Flipper::UI.app(Flipper.instance) do |builder|
+ builder.use Rack::Auth::Basic do |_, password|
+ ActiveSupport::SecurityUtils.secure_compare(
+ ::Digest::SHA256.hexdigest(password),
+ ::Digest::SHA256.hexdigest(ENV["FEATURES_ADMIN_PASSWORD"]),
+ )
+ end
+ end
+ mount flipper_app, at: "/features"
+
  get "/auth/github/callback", to: "sessions#create"
  get "/sign_out", to: "sessions#destroy"
  get "/configuration", to: redirect(ENV.fetch("DOCS_URL"), status: 302)

db/migrate/20200401183732_create_flipper_tables.rb

@@ -0,0 +1,22 @@
+class CreateFlipperTables < ActiveRecord::Migration[6.0]
+ def self.up
+ create_table :flipper_features do |t|
+ t.string :key, null: false
+ t.timestamps null: false
+ end
+ add_index :flipper_features, :key, unique: true
+
+ create_table :flipper_gates do |t|
+ t.string :feature_key, null: false
+ t.string :key, null: false
+ t.string :value
+ t.timestamps null: false
+ end
+ add_index :flipper_gates, [:feature_key, :key, :value], unique: true
+ end
+
+ def self.down
+ drop_table :flipper_gates
+ drop_table :flipper_features
+ end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_12_12_043041) do
+ActiveRecord::Schema.define(version: 2020_04_01_183732) do
 
  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"
@@ -62,7 +62,23 @@
  t.index ["build_id"], name: "index_file_reviews_on_build_id"
  end
 
- create_table "memberships", id: :serial, force: :cascade do |t|
+ create_table "flipper_features", force: :cascade do |t|
+ t.string "key", null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["key"], name: "index_flipper_features_on_key", unique: true
+ end
+
+ create_table "flipper_gates", force: :cascade do |t|
+ t.string "feature_key", null: false
+ t.string "key", null: false
+ t.string "value"
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true
+ end
+
+ create_table "memberships", force: :cascade do |t|
  t.integer "user_id", null: false
  t.integer "repo_id", null: false
  t.datetime "created_at", null: false

spec/services/suggest_changes_spec.rb

@@ -4,89 +4,127 @@
 
 RSpec.describe SuggestChanges do
  describe ".call" do
- context "single violation" do
- it "makes a suggestion for a missing comma" do
- violation = instance_double(
- "Violation",
- messages: [
- "Put a comma after the last parameter of a multiline method call.",
- ],
- source: " violation.fetch(:source)",
- )
- suggest_changes = described_class.new(violation)
-
- result = suggest_changes.call
-
- expect(result).to eq <<~COMMENT.chomp
- Put a comma after the last parameter of a multiline method call.<br>
- ```suggestion
- violation.fetch(:source),
- ```
- COMMENT
+ context "when suggested changes are enabled" do
+ before do
+ flipper_feature = double(enabled?: true)
+ stub_const("Flipper", { suggested_changes: flipper_feature })
  end
 
- it "makes suggestion for missing semicolon" do
- violation = instance_double(
- "Violation",
- messages: [
- "Missing semicolon semi",
- "Something else",
- ],
- source: " console.log('wat')",
- )
- suggest_changes = described_class.new(violation)
-
- result = suggest_changes.call
-
- expect(result).to eq <<~COMMENT.chomp
- Missing semicolon semi<br>Something else<br>
- ```suggestion
- console.log('wat');
- ```
- COMMENT
+ context "single violation" do
+ it "makes a suggestion for a missing comma" do
+ violation = instance_double(
+ "Violation",
+ messages: [
+ "Put a comma after the last parameter of a multiline method call.",
+ ],
+ source: " violation.fetch(:source)",
+ owner: double,
+ )
+ suggest_changes = described_class.new(violation)
+
+ result = suggest_changes.call
+
+ expect(result).to eq <<~COMMENT.chomp
+ Put a comma after the last parameter of a multiline method call.<br>
+ ```suggestion
+ violation.fetch(:source),
+ ```
+ COMMENT
+ end
+
+ it "makes suggestion for missing semicolon" do
+ violation = instance_double(
+ "Violation",
+ messages: [
+ "Missing semicolon semi",
+ "Something else",
+ ],
+ source: " console.log('wat')",
+ owner: double,
+ )
+ suggest_changes = described_class.new(violation)
+
+ result = suggest_changes.call
+
+ expect(result).to eq <<~COMMENT.chomp
+ Missing semicolon semi<br>Something else<br>
+ ```suggestion
+ console.log('wat');
+ ```
+ COMMENT
+ end
+
+ it "makes suggestion for missing space after comma" do
+ violation = instance_double(
+ "Violation",
+ messages: [
+ "A space is required after ',' comma-spacing",
+ ],
+ source: "function wat(one,two, three) {",
+ owner: double,
+ )
+ suggest_changes = described_class.new(violation)
+
+ result = suggest_changes.call
+
+ expect(result).to eq <<~COMMENT.chomp
+ A space is required after ',' comma-spacing<br>
+ ```suggestion
+ function wat(one, two, three) {
+ ```
+ COMMENT
+ end
  end
 
- it "makes suggestion for missing space after comma" do
- violation = instance_double(
- "Violation",
- messages: [
- "A space is required after ',' comma-spacing",
- ],
- source: "function wat(one,two, three) {",
- )
- suggest_changes = described_class.new(violation)
-
- result = suggest_changes.call
-
- expect(result).to eq <<~COMMENT.chomp
- A space is required after ',' comma-spacing<br>
- ```suggestion
- function wat(one, two, three) {
- ```
- COMMENT
+ context "multiple violations" do
+ it "makes a suggestion for the first fixable violation" do
+ violation = instance_double(
+ "Violation",
+ messages: [
+ "Layout/TrailingWhitespace: Trailing whitespace detected.",
+ "Put a comma after the last parameter of a multiline method call.",
+ ],
+ source: " violation.fetch(:source) ",
+ owner: double,
+ )
+ suggest_changes = described_class.new(violation)
+
+ result = suggest_changes.call
+
+ expect(result).to eq <<~COMMENT.chomp
+ Layout/TrailingWhitespace: Trailing whitespace detected.<br>Put a comma after the last parameter of a multiline method call.<br>
+ ```suggestion
+ violation.fetch(:source)
+ ```
+ COMMENT
+ end
  end
  end
 
- context "multiple violations" do
- it "makes a suggestion for the first fixable violation" do
- violation = instance_double(
- "Violation",
- messages: [
- "Layout/TrailingWhitespace: Trailing whitespace detected.",
- "Put a comma after the last parameter of a multiline method call.",
- ],
- source: " violation.fetch(:source) ",
- )
- suggest_changes = described_class.new(violation)
-
- result = suggest_changes.call
-
- expect(result).to eq <<~COMMENT.chomp
- Layout/TrailingWhitespace: Trailing whitespace detected.<br>Put a comma after the last parameter of a multiline method call.<br>
- ```suggestion
- violation.fetch(:source)
- ```
- COMMENT
+ context "when suggested changes are disabled" do
+ context "single violation" do
+ it "does not make suggestion" do
+ violation = instance_double(
+ "Violation",
+ messages: [
+ "Put a comma after the last parameter of a multiline method call.",
+ ],
+ source: " violation.fetch(:source)",
+ owner: double,
+ )
+ suggest_changes = described_class.new(violation)
+ flipper_feature = double
+ stub_const("Flipper", { suggested_changes: flipper_feature })
+ allow(flipper_feature).to(
+ receive(:enabled?).with(violation.owner).and_return(false)
+ )
+
+ result = suggest_changes.call
+
+ expect(result).to eq <<~COMMENT.chomp
+ Put a comma after the last parameter of a multiline method call.
+ COMMENT
+ end
  end
  end
  end