Simplify relations by not creating a 1:1 relation with BoardFilter

app/models/board_filter.rb

-class BoardFilter < ActiveRecord::Base
-  belongs_to :board
-  belongs_to :milestone
-  belongs_to :author, class_name: 'User'
-  belongs_to :assignee, class_name: 'User'
-
-  has_many :board_filter_labels
-  has_many :labels, through: :board_filter_labels
-
-  validates :board, presence: true
-
-  def milestone
-    return nil unless board.parent.feature_available?(:scoped_issue_board)
-
-    if milestone_id == ::Milestone::Upcoming.id
-      ::Milestone::Upcoming
-    else
-      super
-    end
-  end
-end

app/models/board_filter_label.rb

 class BoardFilterLabel < ActiveRecord::Base
-  belongs_to :board_filter
+  belongs_to :board
   belongs_to :label
 
-  validates :board_filter, presence: true
+  validates :board, presence: true
   validates :label, presence: true
-  validates :board_filter, uniqueness: { scope: :label_id }
 end

db/migrate/20170921154104_create_board_filters_table.rb

-class CreateBoardFiltersTable < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  def up
-    create_table :board_filters do |t|
-      t.integer :board_id, null: false, index: true
-
-      t.integer :milestone_id, index: true
-      t.integer :weight, index: true
-      t.integer :author_id, index: true
-      t.integer :assignee_id, index: true
-    end
-
-    add_concurrent_foreign_key :board_filters, :boards, column: :board_id, on_delete: :cascade
-    add_concurrent_foreign_key :board_filters, :milestones, column: :milestone_id, on_delete: :nullify
-    add_concurrent_foreign_key :board_filters, :users, column: :author_id, on_delete: :nullify
-    add_concurrent_foreign_key :board_filters, :users, column: :assignee_id, on_delete: :nullify
-  end
-
-  def down
-    drop_table :board_filters
-  end
-end

db/migrate/20170921195938_generate_board_filters.rb

-class GenerateBoardFilters < ActiveRecord::Migration
-  DOWNTIME = false
-
-  def up
-    # Sub-query executed on production
-    # https://gitlab.com/gitlab-com/infrastructure/issues/2839#note_41023984
-    execute <<-SQL
-      INSERT INTO board_filters(board_id, milestone_id)
-        SELECT id as board_id, milestone_id FROM boards
-        WHERE (boards.milestone_id IS NOT NULL);
-    SQL
-  end
-
-  def down
-    execute <<-SQL
-      DELETE FROM board_filters;
-    SQL
-  end
-end

db/migrate/20170926200545_add_board_filter_fields.rb

+class AddBoardFilterFields < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :boards, :weight, :integer, index: true
+    add_reference :boards, :author, index: true
+    add_reference :boards, :assignee, index: true
+
+    add_concurrent_foreign_key :boards, :users, column: :author_id, on_delete: :nullify
+    add_concurrent_foreign_key :boards, :users, column: :assignee_id, on_delete: :nullify
+  end
+
+  def down
+    remove_columns :boards, :weight, :author_id, :assignee_id
+  end
+end

db/migrate/20170926202731_add_milestone_foreign_key_to_boards.rb

+class AddMilestoneForeignKeyToBoards < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :boards, :milestones, column: :milestone_id, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key :boards, column: :milestone_id
+  end
+end

db/migrate/20170921203824_create_board_filter_labels.rb → db/migrate/20170926203418_create_board_filter_labels.rb

@@ -7,13 +7,13 @@ class CreateBoardFilterLabels < ActiveRecord::Migration
 
   def up
     create_table :board_filter_labels do |t|
-      t.integer :board_filter_id, null: false, index: true
+      t.integer :board_id, null: false, index: true
       t.integer :label_id, null: false, index: true
     end
 
-    add_index :board_filter_labels, [:board_filter_id, :label_id], unique: true
+    add_index :board_filter_labels, [:board_id, :label_id], unique: true
 
-    add_concurrent_foreign_key :board_filter_labels, :board_filters, column: :board_filter_id, on_delete: :cascade
+    add_concurrent_foreign_key :board_filter_labels, :boards, column: :board_id, on_delete: :cascade
     add_concurrent_foreign_key :board_filter_labels, :labels, column: :label_id, on_delete: :cascade
   end
 

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170921203824) do
+ActiveRecord::Schema.define(version: 20170926203418) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -215,28 +215,14 @@ ActiveRecord::Schema.define(version: 20170921203824) do
   add_index "award_emoji", ["user_id", "name"], name: "index_award_emoji_on_user_id_and_name", using: :btree
 
   create_table "board_filter_labels", force: :cascade do |t|
-    t.integer "board_filter_id", null: false
+    t.integer "board_id", null: false
     t.integer "label_id", null: false
   end
 
-  add_index "board_filter_labels", ["board_filter_id", "label_id"], name: "index_board_filter_labels_on_board_filter_id_and_label_id", unique: true, using: :btree
-  add_index "board_filter_labels", ["board_filter_id"], name: "index_board_filter_labels_on_board_filter_id", using: :btree
+  add_index "board_filter_labels", ["board_id", "label_id"], name: "index_board_filter_labels_on_board_id_and_label_id", unique: true, using: :btree
+  add_index "board_filter_labels", ["board_id"], name: "index_board_filter_labels_on_board_id", using: :btree
   add_index "board_filter_labels", ["label_id"], name: "index_board_filter_labels_on_label_id", using: :btree
 
-  create_table "board_filters", force: :cascade do |t|
-    t.integer "board_id", null: false
-    t.integer "milestone_id"
-    t.integer "weight"
-    t.integer "author_id"
-    t.integer "assignee_id"
-  end
-
-  add_index "board_filters", ["assignee_id"], name: "index_board_filters_on_assignee_id", using: :btree
-  add_index "board_filters", ["author_id"], name: "index_board_filters_on_author_id", using: :btree
-  add_index "board_filters", ["board_id"], name: "index_board_filters_on_board_id", using: :btree
-  add_index "board_filters", ["milestone_id"], name: "index_board_filters_on_milestone_id", using: :btree
-  add_index "board_filters", ["weight"], name: "index_board_filters_on_weight", using: :btree
-
   create_table "boards", force: :cascade do |t|
     t.integer "project_id"
     t.datetime "created_at", null: false
@@ -244,8 +230,13 @@ ActiveRecord::Schema.define(version: 20170921203824) do
     t.string "name", default: "Development", null: false
     t.integer "milestone_id"
     t.integer "group_id"
+    t.integer "weight"
+    t.integer "author_id"
+    t.integer "assignee_id"
   end
 
+  add_index "boards", ["assignee_id"], name: "index_boards_on_assignee_id", using: :btree
+  add_index "boards", ["author_id"], name: "index_boards_on_author_id", using: :btree
   add_index "boards", ["group_id"], name: "index_boards_on_group_id", using: :btree
   add_index "boards", ["milestone_id"], name: "index_boards_on_milestone_id", using: :btree
   add_index "boards", ["project_id"], name: "index_boards_on_project_id", using: :btree
@@ -2070,14 +2061,13 @@ ActiveRecord::Schema.define(version: 20170921203824) do
 
   add_foreign_key "approvals", "merge_requests", name: "fk_310d714958", on_delete: :cascade
   add_foreign_key "approver_groups", "namespaces", column: "group_id", on_delete: :cascade
-  add_foreign_key "board_filter_labels", "board_filters", name: "fk_ebc90d2f1a", on_delete: :cascade
+  add_foreign_key "board_filter_labels", "boards", name: "fk_53e44f3a07", on_delete: :cascade
   add_foreign_key "board_filter_labels", "labels", name: "fk_91e18fdcee", on_delete: :cascade
-  add_foreign_key "board_filters", "boards", name: "fk_87e919b0eb", on_delete: :cascade
-  add_foreign_key "board_filters", "milestones", name: "fk_37d28eeebc", on_delete: :nullify
-  add_foreign_key "board_filters", "users", column: "assignee_id", name: "fk_e7893dfa6e", on_delete: :nullify
-  add_foreign_key "board_filters", "users", column: "author_id", name: "fk_b341da4d2b", on_delete: :nullify
+  add_foreign_key "boards", "milestones", name: "fk_b93160e8ee", on_delete: :nullify
   add_foreign_key "boards", "namespaces", column: "group_id", name: "fk_1e9a074a35", on_delete: :cascade
   add_foreign_key "boards", "projects", name: "fk_f15266b5f9", on_delete: :cascade
+  add_foreign_key "boards", "users", column: "assignee_id", name: "fk_2a3450e77c", on_delete: :nullify
+  add_foreign_key "boards", "users", column: "author_id", name: "fk_58e8fc64f3", on_delete: :nullify
   add_foreign_key "chat_teams", "namespaces", on_delete: :cascade
   add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify
   add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade

ee/app/models/ee/board.rb

@@ -3,9 +3,13 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      has_one :board_filter
-
       belongs_to :group
+      belongs_to :milestone
+      belongs_to :author, class_name: 'User'
+      belongs_to :assignee, class_name: 'User'
+
+      has_many :board_filter_labels
+      has_many :labels, through: :board_filter_labels
 
       validates :name, presence: true
       validates :group, presence: true, unless: :project
@@ -23,6 +27,16 @@ module EE
       group_id.present?
     end
 
+    def milestone
+      return nil unless parent.feature_available?(:scoped_issue_board)
+
+      if milestone_id == ::Milestone::Upcoming.id
+        ::Milestone::Upcoming
+      else
+        super
+      end
+    end
+
     def as_json(options = {})
       milestone_attrs = options.fetch(:include, {})
                           .extract!(:milestone)

spec/ee/spec/models/ee/board_spec.rb

 require 'spec_helper'
 
 describe Board do
+  let(:board) { create(:board) }
+
+  it { is_expected.to include_module(EE::Board) }
+
   context 'validations' do
     context 'when group is present' do
       subject { described_class.new(group: create(:group)) }
@@ -16,4 +20,39 @@ describe Board do
       it { is_expected.not_to validate_presence_of(:group) }
     end
   end
+
+  describe 'milestone' do
+    context 'when the feature is available' do
+      before do
+        stub_licensed_features(scoped_issue_board: true)
+      end
+
+      it 'returns Milestone::Upcoming for upcoming milestone id' do
+        board.milestone_id = Milestone::Upcoming.id
+
+        expect(board.milestone).to eq Milestone::Upcoming
+      end
+
+      it 'returns milestone for valid milestone id' do
+        milestone = create(:milestone)
+        board.milestone_id = milestone.id
+
+        expect(board.milestone).to eq milestone
+      end
+
+      it 'returns nil for invalid milestone id' do
+        board.milestone_id = -1
+
+        expect(board.milestone).to be_nil
+      end
+    end
+
+    it 'returns nil when the feature is not available' do
+      stub_licensed_features(scoped_issue_board: false)
+      milestone = create(:milestone)
+      board.milestone_id = milestone.id
+
+      expect(board.milestone).to be_nil
+    end
+  end
 end

spec/factories/board_filter_labels.rb

 FactoryGirl.define do
   factory :board_filter_label do
-    association :board_filter
+    association :board
     association :label
   end
 end

spec/factories/board_filters.rb

-FactoryGirl.define do
-  factory :board_filter do
-    association :board
-    association :milestone
-    association :author, factory: :user
-    association :assignee, factory: :user
-  end
-end

app/models/board_filter_label_spec.rb → spec/models/board_filter_label_spec.rb

 require 'spec_helper'
 
-describe BoardFilterLabel, type: :model do
+describe BoardFilterLabel do
   describe 'validations' do
-    subject { create(:board_filter_label) }
-
-    it { is_expected.to validate_presence_of(:board_filter) }
+    it { is_expected.to validate_presence_of(:board) }
     it { is_expected.to validate_presence_of(:label) }
-    it { is_expected.to validate_uniqueness_of(:board_filter).scoped_to(:label_id) }
   end
 
   describe 'associations' do
-    it { is_expected.to belong_to(:board_filter) }
     it { is_expected.to belong_to(:label) }
   end
 end
\ No newline at end of file

spec/models/board_filter_spec.rb

-require 'spec_helper'
-
-describe BoardFilter, type: :model do
-  describe 'validations' do
-    it { is_expected.to validate_presence_of(:board) }
-  end
-
-  describe 'associations' do
-    it { is_expected.to belong_to(:board) }
-    it { is_expected.to belong_to(:milestone) }
-    it { is_expected.to belong_to(:author).class_name('User') }
-    it { is_expected.to belong_to(:assignee).class_name('User') }
-    it { is_expected.to have_many(:board_filter_labels) }
-    it { is_expected.to have_many(:labels).through(:board_filter_labels) }
-  end
-
-  describe 'milestone' do
-    subject(:board_filter) { build(:board_filter) }
-
-    context 'when the feature is available' do
-      before do
-        stub_licensed_features(scoped_issue_board: true)
-      end
-
-      it 'returns Milestone::Upcoming for upcoming milestone id' do
-        board_filter.milestone_id = Milestone::Upcoming.id
-
-        expect(board_filter.milestone).to eq Milestone::Upcoming
-      end
-
-      it 'returns milestone for valid milestone id' do
-        milestone = create(:milestone)
-        board_filter.milestone_id = milestone.id
-
-        expect(board_filter.milestone).to eq milestone
-      end
-
-      it 'returns nil for invalid milestone id' do
-        board_filter.milestone_id = -1
-
-        expect(board_filter.milestone).to be_nil
-      end
-    end
-
-    it 'returns nil when the feature is not available' do
-      stub_licensed_features(scoped_issue_board: false)
-      milestone = create(:milestone)
-      board_filter.milestone_id = milestone.id
-
-      expect(board_filter.milestone).to be_nil
-    end
-  end
-end

spec/models/board_spec.rb

@@ -4,6 +4,11 @@ describe Board do
   describe 'relationships' do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:milestone) }
+    it { is_expected.to belong_to(:author).class_name('User') }
+    it { is_expected.to belong_to(:assignee).class_name('User') }
+    it { is_expected.to have_many(:board_filter_labels) }
+    it { is_expected.to have_many(:labels).through(:board_filter_labels) }
+
 
     it { is_expected.to have_many(:lists).order(list_type: :asc, position: :asc).dependent(:delete_all) }
   end