Strategy Categories cleanup

zeeshansarwar38 · Feb 25, 2020 · b5def03 · b5def03
1 parent bd58ad1
commit b5def03
Show file tree

Hide file tree

Showing 15 changed files with 34 additions and 109 deletions.
diff --git a/app/controllers/concerns/shared.rb b/app/controllers/concerns/shared.rb
@@ -34,22 +34,20 @@ def shared_update(model_object, model_params)
   end
 
   def shared_destroy(model_object)
-    temp_remove_model_objects(model_object)
-    if model_object.class == Category
-      current_user.strategies.each do |s|
-        update_object(model_object, s)
-      end
-    end
+    remove_model_objects(model_object)
     model_object.destroy
     redirect_to_path(index_path(model_object))
   end
 
   private
 
-  def temp_remove_model_objects(model_object)
+  def remove_model_objects(model_object)
     return if [Mood, Category, Strategy].include?(model_object.class)
 
     current_user.moments.each { |m| update_object(model_object, m) }
+    current_user.strategies.each do |s|
+      update_object(model_object, s) if model_object.class == Category
+    end
   end
 
   def index_path(model_object)

diff --git a/app/controllers/strategies_controller.rb b/app/controllers/strategies_controller.rb
@@ -123,8 +123,8 @@ def set_strategy
 
   def strategy_params
     params.require(:strategy).permit(
-      :name, :description, :published_at, :draft, :comment, { category: [] },
-      { viewers: [] }, perform_strategy_reminder_attributes: %i[active id]
+      :name, :description, :published_at, :draft, :comment, category: [],
+      viewers: [], perform_strategy_reminder_attributes: %i[active id]
     )
   end
 

diff --git a/app/helpers/most_focus_helper.rb b/app/helpers/most_focus_helper.rb
@@ -26,12 +26,10 @@ def viewable?(item)
   def get_data_objs(item, data_type)
     if data_type == 'moods'
       item.moods.pluck(:id)
-    elsif data_type == 'categories' && item.is_a?(Moment)
+    elsif data_type == 'categories'
       item.categories.pluck(:id)
     elsif data_type == 'strategies'
       item.strategies.pluck(:id)
-    else
-      item[data_type]
     end
   end
 

diff --git a/app/helpers/strategies_form_helper.rb b/app/helpers/strategies_form_helper.rb
@@ -100,7 +100,7 @@ def category_checkboxes
         id: item.slug,
         label: item.name,
         value: item.id,
-        checked: @strategy.category.include?(item.id)
+        checked: @strategy.categories.include?(item)
       )
     end
     checkboxes

diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb
@@ -70,12 +70,10 @@ def get_total(result)
   def tagged_data_result(tag, data)
     if tag.is_a?(Mood)
       get_moods_from_data(data, tag)
-    elsif tag.is_a?(Category) && data.first.is_a?(Moment)
+    elsif tag.is_a?(Category)
       get_categories_from_data(data, tag)
     elsif tag.is_a?(Strategy)
       get_strategies_from_data(data, tag)
-    else
-      get_attribute_from_data(data, tag)
     end
   end
 

diff --git a/app/helpers/viewers_helper.rb b/app/helpers/viewers_helper.rb
@@ -43,13 +43,9 @@ def get_viewer_list(data, link)
   end
 
   def get_viewers(data, data_type, obj)
-    data_types = %w[moods categories strategies]
     objs = obj.where(user_id: data.user_id).all.order('created_at DESC')
     objs.each do |ob|
-      item = ob.send(ob.is_a?(Strategy) ? data_type.singularize : data_type)
-      if !ob.is_a?(Strategy) && data_types.include?(data_type)
-        item = item.pluck(:id)
-      end
+      item = ob.send(data_type).pluck(:id)
       return ob.viewers if item.include?(data.id)
     end
     []

diff --git a/app/models/concerns/common_methods.rb b/app/models/concerns/common_methods.rb
@@ -5,22 +5,13 @@ module CommonMethods
   def mood_names_and_slugs
     return unless self.class.reflect_on_association(:moods)
 
-    names_and_slugs_hash(
-      moods.pluck(:name, :slug),
-      'moods'
-    )
+    names_and_slugs_hash(moods.pluck(:name, :slug), 'moods')
   end
 
   def category_names_and_slugs
-    # TODO: Remove usage of Category when Strategy Categories is created
-    if is_a?(Strategy) && attribute(:category)
-      names_and_slugs_hash(
-        Category.where(id: category).pluck(:name, :slug),
-        'categories'
-      )
-    elsif self.class.reflect_on_association(:categories)
-      names_and_slugs_hash(categories.pluck(:name, :slug), 'categories')
-    end
+    return unless self.class.reflect_on_association(:categories)
+
+    names_and_slugs_hash(categories.pluck(:name, :slug), 'categories')
   end
 
   private

diff --git a/app/models/strategy.rb b/app/models/strategy.rb
@@ -5,7 +5,6 @@
 #
 #  id           :bigint           not null, primary key
 #  user_id      :integer
-#  category     :text
 #  description  :text
 #  viewers      :text
 #  comment      :boolean
@@ -22,7 +21,6 @@ class Strategy < ApplicationRecord
   extend FriendlyId
 
   friendly_id :name
-  serialize :category, Array
   serialize :viewers, Array
 
   before_save :category_array_data
@@ -45,19 +43,20 @@ class Strategy < ApplicationRecord
 
   has_many :moments_strategies, dependent: :destroy
 
+  attr_accessor :category
+
   def active_reminders
     [perform_strategy_reminder].select(&:active?) if perform_strategy_reminder
   end
 
   def viewers_array_data
-    viewers.map!(&:to_i)
+    self.viewers = viewers.collect(&:to_i) if viewers.is_a?(Array)
   end
 
   def category_array_data
     return unless category.is_a?(Array)
 
     category_ids = category.collect(&:to_i)
-    self.category = category_ids
     self.categories = Category.where(user_id: user_id, id: category_ids)
   end
 
@@ -68,11 +67,4 @@ def published?
   def comments
     Comment.comments_from(self)
   end
-
-  def self.populate_strategies_categories
-    Strategy.all.find_each do |strategy|
-      strategy.category = Category.where(id: strategy.category).pluck(:id)
-      strategy.save
-    end
-  end
 end
diff --git a/db/migrate/20200225010346_drop_strategies_category_column.rb b/db/migrate/20200225010346_drop_strategies_category_column.rb
@@ -0,0 +1,5 @@
+class DropStrategiesCategoryColumn < ActiveRecord::Migration[5.2]
+  def change
+    remove_column :strategies, :category
+  end
+end
diff --git a/db/schema.rb b/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: 2020_02_25_003308) do
+ActiveRecord::Schema.define(version: 2020_02_25_010346) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -214,7 +214,6 @@
 
   create_table "strategies", force: :cascade do |t|
     t.integer "user_id"
-    t.text "category"
     t.text "description"
     t.text "viewers"
     t.boolean "comment"

diff --git a/lib/tasks/cleaner.rake b/lib/tasks/cleaner.rake
@@ -13,9 +13,4 @@ namespace :cleaner do
       end
     end
   end
-
-  desc 'Convert existing Straregy category IDs to join table records'
-  task populate_strategies_categories: :environment do
-    Strategy.populate_strategies_categories
-  end
 end
diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb
@@ -198,13 +198,14 @@
       end
       it 'removes categories from existing strategies' do
         delete :destroy, params: { id: category.id }
-        expect(strategy.reload.category).not_to include(category.id)
+        expect(strategy.reload.categories).not_to include(category.id)
       end
       it 'redirects to the category index page' do
         delete :destroy, params: { id: category.id }
         expect(response).to redirect_to categories_path
       end
     end
+
     context 'when the user is not logged in' do
       before { delete :destroy, params: { id: category.id } }
       it_behaves_like :with_no_logged_in_user

diff --git a/spec/controllers/strategies_controller_spec.rb b/spec/controllers/strategies_controller_spec.rb
@@ -233,25 +233,29 @@
       include_context :logged_in_user
 
       context 'when the params are valid' do
-        it 'updates the strategy record' do
+        before(:each) do
           patch :update, params: { id: strategy.id, strategy: valid_strategy_params }
+        end
+
+        it 'updates the strategy record' do
           expect(strategy.reload.description).to eq('updated description')
         end
 
         it 'redirects to the show page' do
-          patch :update, params: { id: strategy.id, strategy: valid_strategy_params }
           expect(response).to redirect_to(strategy_path(strategy))
         end
       end
 
       context 'when the params are invalid' do
-        it 'does not update the record' do
+        before(:each) do
           patch :update, params: { id: strategy.id, strategy: invalid_strategy_params }
+        end
+
+        it 'does not update the record' do
           expect(strategy.reload.description).to eq('Test Description')
         end
 
         it 'renders the edit view' do
-          patch :update, params: { id: strategy.id, strategy: invalid_strategy_params }
           expect(response).to render_template('edit')
         end
       end

diff --git a/spec/helpers/shared_examples.rb b/spec/helpers/shared_examples.rb
@@ -14,8 +14,6 @@
     data_type_name = 'categories'
   elsif data_type == :strategy
     data_type_name = 'strategies'
-  else
-    data_type_name = data_type.to_s
   end
 
   before do

diff --git a/spec/models/strategy_spec.rb b/spec/models/strategy_spec.rb
@@ -25,7 +25,6 @@
   end
 
   context 'with serialize' do
-    it { is_expected.to serialize(:category) }
     it { is_expected.to serialize(:viewers) }
   end
 
@@ -100,59 +99,10 @@
     end
   end
 
-  describe '.populate_strategies_categories' do
-    let(:user) { create(:user) }
-    let(:user_two) { create(:user) }
-
-    let(:category) { create(:category, user: user) }
-    let(:category_two) { create(:category, user: user) }
-    let(:category_three) { create(:category, user: user_two) }
-
-    let!(:strategy) { create(:strategy, user: user, category: [category.id]) }
-    let!(:strategy_two) {
-      create(:strategy, user: user, category: [category.id, category_two.id]) }
-    let!(:strategy_three) {
-      create(:strategy, user: user_two, category: [category_three.id]) }
-    let!(:strategy_four) { create(:strategy, user: user_two) }
-
-    it 'creates join table records' do
-      # Must delete join table records because they're automatically saved
-      ActiveRecord::Base.connection.execute("DELETE from strategies_categories")
-      expect(strategy.categories.count).to eq(0)
-      expect(strategy_two.categories.count).to eq(0)
-      expect(strategy_three.categories.count).to eq(0)
-      expect(strategy_four.categories.count).to eq(0)
-
-      Strategy.populate_strategies_categories
-
-      expect(strategy.categories.count).to eq(1)
-      expect(strategy.categories).to include category
-
-      expect(strategy_two.categories.count).to eq(2)
-      expect(strategy_two.categories).to include category
-      expect(strategy_two.categories).to include category_two
-
-      expect(strategy_three.categories.count).to eq(1)
-      expect(strategy_three.categories).to include category_three
-
-      expect(strategy_four.categories.count).to eq(0)
-    end
-  end
-
   describe '#category_array_data' do
     let!(:strategy) { create(:strategy) }
     let!(:category) { create(:category, user: strategy.user) }
 
-    context 'saving categories as IDs' do
-      it 'sets categories on field upon save' do
-        expect(strategy.category).to eq([])
-        strategy.category = [category.id.to_s]
-        expect {
-          strategy.save
-        }.to change{ strategy.category }.to([category.id])
-      end
-    end
-
     context 'saving categories via relation' do
       let!(:category_two) { create(:category, user: strategy.user) }