Add validations for miq_widget_set

app/models/miq_group.rb

@@ -282,6 +282,18 @@ def self.display_name(number = 1)
     n_('Group', 'Groups', number)
   end
 
+  def delete_from_dashboard_order(widget_set_id)
+    return if settings.blank?
+
+    settings[:dashboard_order]&.delete(widget_set_id)
+  end
+
+  def add_to_dashboard_order(widget_set_id)
+    self.settings ||= {:dashboard_order => []}
+    self.settings[:dashboard_order] ||= []
+    self.settings[:dashboard_order] |= [widget_set_id]
+  end
+
   private
 
   # if this tenant is changing, make sure this is not a default group

app/models/miq_widget_set.rb

@@ -1,8 +1,23 @@
 class MiqWidgetSet < ApplicationRecord
+  include_concern 'SetData'
+
   acts_as_miq_set
 
+  before_destroy :ensure_can_be_destroyed
   before_destroy :destroy_user_versions
+  before_destroy :delete_from_dashboard_order
+
+  after_save     :add_to_dashboard_order
   before_save    :keep_group_when_saving
+  after_save     :update_members
+
+  validates :group_id, :presence => true, :unless => :read_only?
+
+  validates :name, :format => {:with => /\A[^|]*\z/, :on => :create, :message => "cannot contain \"|\""}
+
+  validates :description, :uniqueness_when_changed => {:scope   => :owner_id,
+                                                       :message => _("Description (Tab Title) must be unique for this group")}
+  belongs_to :group, :class_name => 'MiqGroup'
 
   WIDGET_DIR =  File.expand_path(File.join(Rails.root, "product/dashboard/dashboards"))
 
@@ -14,6 +29,31 @@ def self.with_users
     where.not(:userid => nil)
   end
 
+  def update_members
+    replace_children(Array(set_data_widgets)) if members.map(&:id) != set_data_widgets.ids
+    current_user = User.current_user
+    members.each { |w| w.create_initial_content_for_user(current_user.userid) } if current_user # Generate content if not there
+  end
+
+  def add_to_dashboard_order
+    return unless group_id
+
+    group.add_to_dashboard_order(id)
+    group.save
+  end
+
+  def delete_from_dashboard_order
+    return unless group_id
+
+    group = MiqGroup.find(group_id)
+    group.delete_from_dashboard_order(id)
+    group.save
+  end
+
+  def ensure_can_be_destroyed
+    errors.add(:base, _("Unable to delete read-only WidgetSet")) if read_only?
+  end
+
   def destroy_user_versions
     # userid, group_id and name are set for user version
     # group_id, owner_type and owner_id are set for group version
@@ -88,6 +128,7 @@ def self.copy_dashboard(source_widget_set, destination_name, destination_descrip
                          :owner_type  => "MiqGroup",
                          :set_type    => source_widget_set.set_type,
                          :set_data    => source_widget_set.set_data,
+                         :group_id    => assign_to_group.id,
                          :owner_id    => assign_to_group.id)
   end
 

app/models/miq_widget_set/set_data.rb

@@ -0,0 +1,41 @@
+module MiqWidgetSet::SetData
+  extend ActiveSupport::Concern
+
+  SET_DATA_COLS = %i[col1 col2 col3].freeze
+
+  included do
+    validate :set_data do
+      errors.add(:set_data, "One widget must be selected(set_data)") if widget_ids.empty?
+
+      filtered_widget_ids = set_data_widgets.pluck(:id)
+      if (widget_ids - filtered_widget_ids).present?
+        errors.add(:set_data, "Unable to find widget ids: #{(widget_ids - filtered_widget_ids).join(', ')}")
+      end
+    end
+
+    before_validation :init_set_data
+
+    def set_data_widgets
+      MiqWidget.where(:id => widget_ids)
+    end
+
+    private
+
+    def init_set_data
+      self.set_data ||= {}
+      new_set_data ||= {}
+      self.set_data.symbolize_keys!
+      SET_DATA_COLS.each do |col_key|
+        new_set_data[col_key] = self.set_data[col_key] || []
+      end
+
+      new_set_data[:reset_upon_login] = !!set_data[:reset_upon_login]
+      new_set_data[:locked] = !!set_data[:locked]
+      self.set_data = new_set_data
+    end
+
+    def widget_ids
+      SET_DATA_COLS.map { |x| set_data[x] }.flatten.compact
+    end
+  end
+end

spec/factories/miq_widget_set.rb

@@ -2,5 +2,17 @@
   factory :miq_widget_set do
     sequence(:name)         { |n| "widget_set_#{seq_padded_for_sorting(n)}" }
     sequence(:description)  { |n| "widget_set_#{seq_padded_for_sorting(n)}" }
+
+    trait :set_data_with_one_widget do
+      set_data do
+        {:col1             => [FactoryBot.create(:miq_widget).id],
+         :reset_upon_login => false,
+         :locked           => false}
+      end
+    end
+
+    before(:create) do |x|
+      x.group_id = x.owner_id if x.owner_id
+    end
   end
 end

spec/models/miq_group_spec.rb

@@ -317,25 +317,25 @@
   context "#ordered_widget_sets" do
     let(:group) { FactoryBot.create(:miq_group) }
     it "uses dashboard_order if present" do
-      ws1 = FactoryBot.create(:miq_widget_set, :name => 'A1', :owner => group)
-      FactoryBot.create(:miq_widget_set, :name => 'C3', :owner => group)
-      ws3 = FactoryBot.create(:miq_widget_set, :name => 'B2', :owner => group)
+      ws1 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'A1', :owner => group)
+      FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'C3', :owner => group)
+      ws3 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'B2', :owner => group)
       group.update(:settings => {:dashboard_order => [ws3.id.to_s, ws1.id.to_s]})
 
       expect(group.ordered_widget_sets).to eq([ws3, ws1])
     end
 
     it "uses all owned widgets" do
-      ws1 = FactoryBot.create(:miq_widget_set, :name => 'A1', :owner => group)
-      ws2 = FactoryBot.create(:miq_widget_set, :name => 'C3', :owner => group)
-      ws3 = FactoryBot.create(:miq_widget_set, :name => 'B2', :owner => group)
+      ws1 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'A1', :owner => group)
+      ws2 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'C3', :owner => group)
+      ws3 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'B2', :owner => group)
       expect(group.ordered_widget_sets).to eq([ws1, ws3, ws2])
     end
 
     it "works when settings use strings" do
-      ws1 = FactoryBot.create(:miq_widget_set, :name => 'A1', :owner => group)
-      _ws2 = FactoryBot.create(:miq_widget_set, :name => 'C3', :owner => group)
-      ws3 = FactoryBot.create(:miq_widget_set, :name => 'B2', :owner => group)
+      ws1 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'A1', :owner => group)
+      _ws2 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'C3', :owner => group)
+      ws3 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'B2', :owner => group)
       group.update(:settings => {"dashboard_order" => [ws3.id.to_s, ws1.id.to_s]})
 
       expect(group.ordered_widget_sets).to eq([ws3, ws1])

spec/models/miq_widget_set_spec.rb

@@ -2,7 +2,49 @@
   let(:group) { user.current_group }
   let(:user)  { FactoryBot.create(:user_with_group) }
   before do
-    @ws_group = FactoryBot.create(:miq_widget_set, :name => 'Home', :owner => group)
+    @ws_group = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'Home', :owner => group)
+  end
+
+  describe "validate" do
+    it "validates that MiqWidgetSet#name cannot contain \"|\" " do
+      widget_set = MiqWidgetSet.create(:name => 'TEST|TEST')
+
+      expect(widget_set.errors.messages).to include(:name => ["cannot contain \"|\""])
+    end
+
+    let(:other_group) { FactoryBot.create(:miq_group) }
+
+    it "validates that MiqWidgetSet has unique description inside group" do
+      widget_set = MiqWidgetSet.create(:description => @ws_group.description, :owner => group)
+      expect(widget_set.errors.messages).to include(:description => ["Description (Tab Title) must be unique for this group"])
+
+      widget_set = MiqWidgetSet.create(:description => @ws_group.description, :owner => nil)
+      expect(widget_set.errors.messages).not_to include(:description => ["Description (Tab Title) must be unique for this group"])
+
+      widget_set = MiqWidgetSet.create(:description => @ws_group.description, :owner => other_group)
+      expect(widget_set.errors.messages).not_to include(:description => ["Description (Tab Title) must be unique for this group"])
+    end
+
+    it "validates that there is at least one widget in set_data" do
+      widget_set = MiqWidgetSet.create
+
+      expect(widget_set.errors.messages).to include(:set_data => ["One widget must be selected(set_data)"])
+    end
+
+    it "validates that widgets in set_data have to exist" do
+      unknown_id = MiqWidgetSet.maximum(:id) + 1
+      widget_set = MiqWidgetSet.create(:set_data => {:col1 => [unknown_id]})
+
+      expect(widget_set.errors.messages).to include(:set_data => ["Unable to find widget ids: #{unknown_id}"])
+    end
+
+    it "validates that group_id has to be present for non-read_only widget sets" do
+      widget_set = MiqWidgetSet.create(:read_only => false)
+      expect(widget_set.errors.messages).to include(:group_id => ["can't be blank"])
+
+      widget_set = MiqWidgetSet.create(:read_only => true)
+      expect(widget_set.errors.messages).not_to include(:set_data => ["can't be blank"])
+    end
   end
 
   it "when a group dashboard is deleted" do
@@ -22,7 +64,7 @@
 
   context "with a user" do
     before do
-      FactoryBot.create(:miq_widget_set, :name => 'Home', :userid => user.userid, :group_id => group.id)
+      FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'Home', :userid => user.userid, :group_id => group.id)
     end
 
     it "initial state" do
@@ -42,7 +84,7 @@
 
   describe ".destroy_user_versions" do
     before do
-      FactoryBot.create(:miq_widget_set, :name => 'User_Home', :userid => user.userid)
+      FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'User_Home', :userid => user.userid, :owner => group)
     end
 
     it "destroys all user's versions of dashboards (dashboards been customized by user)" do
@@ -55,12 +97,12 @@
 
   describe "#where_unique_on" do
     let(:group2) { FactoryBot.create(:miq_group, :description => 'dev group2') }
-    let(:ws_1)   { FactoryBot.create(:miq_widget_set, :name => 'Home', :userid => user.userid, :group_id => group.id) }
+    let(:ws_1)   { FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'Home', :userid => user.userid, :group_id => group.id) }
 
     before do
       user.miq_groups << group2
       ws_1
-      FactoryBot.create(:miq_widget_set, :name => 'Home', :userid => user.userid, :group_id => group2.id)
+      FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'Home', :userid => user.userid, :group_id => group2.id)
     end
 
     it "initial state" do
@@ -78,26 +120,28 @@
 
   describe "#with_users" do
     it "brings back records with users" do
-      ws_1 = FactoryBot.create(:miq_widget_set, :name => 'Home', :userid => user.userid, :group_id => group.id)
-      expect(described_class.with_users).to eq([ws_1])
+      ws = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :name => 'Home', :userid => user.userid, :group_id => group.id)
+      expect(described_class.with_users).to eq([ws])
     end
   end
 
   context ".find_with_same_order" do
     it "returns in index order" do
-      g1 = FactoryBot.create(:miq_widget_set)
-      g2 = FactoryBot.create(:miq_widget_set)
+      g1 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :read_only => true)
+      g2 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :read_only => true)
       expect(MiqWidgetSet.find_with_same_order([g1.id.to_s, g2.id.to_s])).to eq([g1, g2])
     end
 
     it "returns in non index order" do
-      g1 = FactoryBot.create(:miq_widget_set)
-      g2 = FactoryBot.create(:miq_widget_set)
+      g1 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :read_only => true)
+      g2 = FactoryBot.create(:miq_widget_set, :set_data_with_one_widget, :read_only => true)
       expect(MiqWidgetSet.find_with_same_order([g2.id.to_s, g1.id.to_s])).to eq([g2, g1])
     end
   end
 
   context "loading group specific defaul dashboard" do
+    let!(:miq_widget_set) { FactoryBot.create(:miq_widget, :description => 'chart_vendor_and_guest_os') }
+
     describe ".sync_from_file" do
       let(:dashboard_name) { "Dashboard for Group" }
       before do
@@ -130,11 +174,12 @@
   end
 
   describe ".copy_dashboard" do
-    let(:name) { "New Dashboard Name" }
-    let(:tab) { "Dashboard Tab" }
+    let(:name)        { "New Dashboard Name" }
+    let(:tab)         { "Dashboard Tab" }
+    let(:other_group) { FactoryBot.create(:miq_group) }
 
     it "does not raises error if the same dashboard name used for different groups" do
-      expect { MiqWidgetSet.copy_dashboard(@ws_group, @ws_group.name, tab) }.not_to raise_error
+      expect { MiqWidgetSet.copy_dashboard(@ws_group, @ws_group.name, tab, other_group.id) }.not_to raise_error
     end
 
     it "raises error if passed tab name is empty" do
@@ -159,10 +204,8 @@
     end
 
     it "keeps the same set of widgets and dashboard's settings" do
-      set_data = {:col1 => [1], :col2 => [2], :col3 => [], :locked => false, :reset_upon_login => false, :last_group_db_updated => Time.now.utc}
-      @ws_group.update(:set_data => set_data)
       new_dashboard = MiqWidgetSet.copy_dashboard(@ws_group, name, tab)
-      expect(new_dashboard.set_data).to eq set_data
+      expect(new_dashboard.set_data).to eq @ws_group.set_data
     end
   end
 end