diff --git a/app/models/org.rb b/app/models/org.rb index 12d473f..f97042c 100644 --- a/app/models/org.rb +++ b/app/models/org.rb @@ -73,8 +73,7 @@ validates :name, presence: { message: PRESENCE_MESSAGE }, uniqueness: { message: UNIQUENESS_MESSAGE } - validates :abbreviation, presence: { message: PRESENCE_MESSAGE }, - uniqueness: { message: UNIQUENESS_MESSAGE } + validates :abbreviation, presence: { message: PRESENCE_MESSAGE } validates :is_other, inclusion: { in: BOOLEAN_VALUES, message: INCLUSION_MESSAGE } diff --git a/app/models/section.rb b/app/models/section.rb index 1752930..42cb330 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -112,6 +112,11 @@ return copy end + # Can't be modified as it was duplicatd over from another Phase. + def template? + !modifiable? + end + private # ============================ diff --git a/config/brakeman.ignore b/config/brakeman.ignore index f172a00..ae8ddb0 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -291,6 +291,6 @@ "note": "" } ], - "updated": "2018-08-07 17:41:31 +0100", + "updated": "2018-08-07 17:38:06 +0100", "brakeman_version": "4.3.1" } diff --git a/db/migrate/20180807114035_add_default_value_to_optional_subset_on_guidance_groups.rb b/db/migrate/20180807114035_add_default_value_to_optional_subset_on_guidance_groups.rb new file mode 100644 index 0000000..45f686e --- /dev/null +++ b/db/migrate/20180807114035_add_default_value_to_optional_subset_on_guidance_groups.rb @@ -0,0 +1,8 @@ +class AddDefaultValueToOptionalSubsetOnGuidanceGroups < ActiveRecord::Migration + def up + change_column :guidance_groups, :optional_subset, :boolean, default: false, null: false + end + def down + change_column :guidance_groups, :optional_subset, :boolean, default: nil, null: true + end +end diff --git a/db/migrate/20180807114052_add_default_value_to_published_on_guidance_groups.rb b/db/migrate/20180807114052_add_default_value_to_published_on_guidance_groups.rb new file mode 100644 index 0000000..25006ae --- /dev/null +++ b/db/migrate/20180807114052_add_default_value_to_published_on_guidance_groups.rb @@ -0,0 +1,8 @@ +class AddDefaultValueToPublishedOnGuidanceGroups < ActiveRecord::Migration + def up + change_column :guidance_groups, :published, :boolean, default: false, null: false + end + def down + change_column :guidance_groups, :published, :boolean, default: nil, null: true + end +end diff --git a/db/migrate/20180807120926_add_default_value_to_archived_on_notes.rb b/db/migrate/20180807120926_add_default_value_to_archived_on_notes.rb new file mode 100644 index 0000000..c03e3f5 --- /dev/null +++ b/db/migrate/20180807120926_add_default_value_to_archived_on_notes.rb @@ -0,0 +1,9 @@ +class AddDefaultValueToArchivedOnNotes < ActiveRecord::Migration + def up + change_column :notes, :archived, :boolean, default: false, null: false + end + + def down + change_column :notes, :archived, :boolean, default: nil, null: true + end +end diff --git a/db/migrate/20180807121126_add_default_value_to_is_other_on_orgs.rb b/db/migrate/20180807121126_add_default_value_to_is_other_on_orgs.rb new file mode 100644 index 0000000..3aade3b --- /dev/null +++ b/db/migrate/20180807121126_add_default_value_to_is_other_on_orgs.rb @@ -0,0 +1,8 @@ +class AddDefaultValueToIsOtherOnOrgs < ActiveRecord::Migration + def up + change_column :orgs, :is_other, :boolean, default: false, null: false + end + def down + change_column :orgs, :is_other, :boolean, default: nil, null: true + end +end diff --git a/lib/data_cleanup.rb b/lib/data_cleanup.rb index 985768b..5f105fd 100644 --- a/lib/data_cleanup.rb +++ b/lib/data_cleanup.rb @@ -1,9 +1,13 @@ # frozen_string_literal: true + +require_relative "data_cleanup/model_check" +require_relative "data_cleanup/instance_check" +require_relative "data_cleanup/reporting" +require_relative "data_cleanup/rules" + module DataCleanup - require_relative "data_cleanup/model_check" - require_relative "data_cleanup/instance_check" - require_relative "data_cleanup/reporting" - require_relative "data_cleanup/rules" + + COLOR_CODES = { red: 31, green: 32 } module_function @@ -11,14 +15,9 @@ @logger ||= Logger.new(Rails.root.join("log", "validations.log")) end - COLOR_CODES = { red: 31, green: 32 } - - def logger.info(message, inline: false, color: nil) - message = message + "\n" unless inline - if color - message = "\e[#{COLOR_CODES[color]}m#{message}\e[0m" - end - super(message) unless inline + def display(message, inline: false, color: nil) + message = "#{message}\n" unless inline + message = "\e[#{COLOR_CODES[color]}m#{message}\e[0m" if color print message end end diff --git a/lib/data_cleanup/instance_check.rb b/lib/data_cleanup/instance_check.rb index c7569b8..02d9c5e 100644 --- a/lib/data_cleanup/instance_check.rb +++ b/lib/data_cleanup/instance_check.rb @@ -6,17 +6,23 @@ # frozen_string_literal: true def call(instance) + DataCleanup.logger.info("Checking #{instance.class}##{instance.id}...") Reporting.total_record_count += 1 begin if instance.invalid? + DataCleanup.logger.info(<<~TEXT) + Instance #{instance.class}##{instance.id} invalid! + Errors: #{instance.errors.full_messages.to_sentence} + TEXT Reporting.invalid_record_count += 1 Reporting.invalid_records << instance - DataCleanup.logger.info("F", inline: true) + DataCleanup.display("F", inline: true) else - DataCleanup.logger.info(".", inline: true) + DataCleanup.logger.info("Instance #{instance.class}##{instance.id} valid!") + DataCleanup.display(".", inline: true) end rescue Dragonfly::Job::Fetch::NotFound - DataCleanup.logger.info(".", inline: true) + DataCleanup.display(".", inline: true) end end end diff --git a/lib/data_cleanup/model_check.rb b/lib/data_cleanup/model_check.rb index 1b42446..b1b793b 100644 --- a/lib/data_cleanup/model_check.rb +++ b/lib/data_cleanup/model_check.rb @@ -19,12 +19,12 @@ when 'EXCLUDE' return if model.model_name.in?(models.split(",")) end - DataCleanup.logger.info "Checking #{model.model_name.plural}:" + DataCleanup.display "Checking #{model.model_name.plural}:" model.find_in_batches do |batch| instance_check = InstanceCheck.new batch.each { |instance| instance_check.(instance) } end - DataCleanup.logger.info "" + DataCleanup.display "" end end end diff --git a/lib/data_cleanup/reporting.rb b/lib/data_cleanup/reporting.rb index ebeba3b..50c13e0 100644 --- a/lib/data_cleanup/reporting.rb +++ b/lib/data_cleanup/reporting.rb @@ -24,9 +24,12 @@ end def report - issues_found.each { |issue| DataCleanup.logger.info issue } + issues_found.each do |issue| + DataCleanup.display issue + DataCleanup.logger.info issue + end color = invalid_record_count.zero? ? :green : :red - DataCleanup.logger.info(<<~TEXT, color: color) + DataCleanup.display(<<~TEXT, color: color) Invalid records: #{invalid_record_count} / #{total_record_count} TEXT end diff --git a/lib/data_cleanup/rules/annotation/fix_blank_text.rb b/lib/data_cleanup/rules/annotation/fix_blank_text.rb new file mode 100644 index 0000000..7589096 --- /dev/null +++ b/lib/data_cleanup/rules/annotation/fix_blank_text.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank text on Annotation + module Annotation + + class FixBlankText < Rules::Base + + def description + "Fix blank text on Annotation" + end + + def call + ::Annotation.where(text: "").find_in_batches do |batches| + batches.each do |annotation| + log("Destroying Annotation##{annotation.id} since it has no text") + annotation.destroy + end + end + end + + end + + end + end +end diff --git a/lib/data_cleanup/rules/annotation/fix_duplicate_type.rb b/lib/data_cleanup/rules/annotation/fix_duplicate_type.rb new file mode 100644 index 0000000..be5dd7a --- /dev/null +++ b/lib/data_cleanup/rules/annotation/fix_duplicate_type.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix duplicate type on Annotation + module Annotation + + class FixDuplicateType < Rules::Base + + def description + "Fix duplicate type on Annotation" + end + + def call + ::Annotation.group(:question_id, :type, :org_id) + .count + .select { |k,v| v > 1 } + .each do |array, count| + + question_id = array.first + type = array.second + org_id = array.third + + log("Destroying all duplicate Annotations with question_id: #{question_id} "\ + "type: #{type} and org_id: #{org_id}") + + ::Annotation.where(question_id: question_id, type: type, org_id: org_id) + .order(updated_at: :desc) + .offset(1) + .destroy_all + end + end + + end + + end + end +end diff --git a/lib/data_cleanup/rules/answer/fix_blank_user.rb b/lib/data_cleanup/rules/answer/fix_blank_user.rb index 987834a..a17575d 100644 --- a/lib/data_cleanup/rules/answer/fix_blank_user.rb +++ b/lib/data_cleanup/rules/answer/fix_blank_user.rb @@ -10,8 +10,25 @@ end def call - ::Answer.where(user: nil).destroy_all + ::Answer.joins("LEFT OUTER JOIN users ON users.id = answers.user_id") + .where(users: { id: nil }) + .includes(plan: { roles: :user }).each do |answer| + + if answer.plan.owner.present? + log("Updating Answer##{answer.id} with user: #{answer.plan.owner}") + answer.update(user: answer.plan.owner) + elsif answer.plan.roles.any? + user = answer.plan.roles.first.user + log("Updating Answer##{answer.id} with user: #{user}") + answer.update(user: user) + else + log("Destroying orphaned Answer##{answer.id}") + answer.destroy + end + end + end + end end end diff --git a/lib/data_cleanup/rules/answer/fix_duplicate_question.rb b/lib/data_cleanup/rules/answer/fix_duplicate_question.rb index 300d4a5..d028842 100644 --- a/lib/data_cleanup/rules/answer/fix_duplicate_question.rb +++ b/lib/data_cleanup/rules/answer/fix_duplicate_question.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module DataCleanup module Rules # Fix duplicate question on Answer @@ -15,9 +16,10 @@ .count.select { |k,v| v > 1 } # Values looks like [{ [123, 199] => 2}, ...] dataset.each do |values, count| + log("Destroying all Answers that are duplicates of earlier answers on the same question.") # ... and destroy all duplicates, keeping the latest record ::Answer.where(question: values.first, plan_id: values.last) - .order("created_at DESC") + .order("updated_at DESC") .offset(1) .destroy_all end diff --git a/lib/data_cleanup/rules/base.rb b/lib/data_cleanup/rules/base.rb index cff2d1d..cc828e4 100644 --- a/lib/data_cleanup/rules/base.rb +++ b/lib/data_cleanup/rules/base.rb @@ -3,6 +3,10 @@ # Base class for rules to clean invalid database records class Base + def log(message) + DataCleanup.logger.info(message) + end + # Description of the rule and how it's fixing the data def description self.class.name.humanize @@ -12,6 +16,7 @@ def call raise NotImplementedError, "Please define call() in #{self}" end + end end end diff --git a/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb b/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb index 90d0363..5bb309f 100644 --- a/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb +++ b/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb @@ -11,7 +11,13 @@ end def call - ::ExportedPlan.where(plan: nil).delete_all + # Find all exported plans where the corresponding plan doesn't exist. + ::ExportedPlan + .joins("LEFT OUTER JOIN plans on plans.id = exported_plans.plan_id") + .where(plans: { id: nil }).each do |exported_plan| + log("Destroying ExportedPlan##{exported_plan.id} where plan is nil") + exported_plan.destroy + end end end end diff --git a/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb b/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb deleted file mode 100644 index 9cad432..0000000 --- a/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb +++ /dev/null @@ -1,17 +0,0 @@ -module DataCleanup - module Rules - module GuidanceGroup - class FixBlankOptionalSubset < Rules::Base - - def description - "Fix blank optional subset on GuidanceGroup" - end - - def call - ::GuidanceGroup.where(optional_subset: nil) - .update_all(optional_subset: false) - end - end - end - end -end diff --git a/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb b/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb deleted file mode 100644 index f11b95b..0000000 --- a/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true -module DataCleanup - module Rules - module GuidanceGroup - # Fix blank published on GuidanceGroup - class FixBlankPublished < Rules::Base - - def description - "Fix blank published on GuidanceGroup" - end - - def call - ::GuidanceGroup.where(published: nil).update_all(published: false) - end - end - end - end -end diff --git a/lib/data_cleanup/rules/note/fix_blank_archived.rb b/lib/data_cleanup/rules/note/fix_blank_archived.rb deleted file mode 100644 index 0f12339..0000000 --- a/lib/data_cleanup/rules/note/fix_blank_archived.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true -module DataCleanup - module Rules - # Fix blank archived on Note - module Note - class FixBlankArchived < Rules::Base - - def description - "Fix blank archived on Note" - end - - def call - ::Note.where(archived: nil).update_all(archived: false) - end - end - end - end -end diff --git a/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb b/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb index 40354b6..89c999b 100644 --- a/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb +++ b/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb @@ -13,9 +13,11 @@ def call if File.exists?(YAML_FILE_PATH) YAML.load_file(YAML_FILE_PATH).each do |attributes| - attributes = attributes.with_indifferent_access - ::Org.where(name: attributes["name"], id: attributes['id']) - .update_all(abbreviation: attributes['abbreviation']) + attributes = attributes.with_indifferent_access + name = attributes['name'] + abbreviation = attributes['abbreviation'] + log("Adding abbreviation #{abbreviation} to Org '#{name}'") + ::Org.where(name: name).update_all(abbreviation: abbreviation) end else raise "Please create a YAML file at #{YAML_FILE_PATH}" diff --git a/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb b/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb index cefdc38..bc1bcac 100644 --- a/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb +++ b/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb @@ -13,10 +13,12 @@ HTML def description - "Fix orgs where feedback_enabled is true" + "Fix orgs feedback_email_message is blank" end def call + ids = ::Org.where(feedback_enabled: true, feedback_email_msg: "").pluck(:id) + log("Adding default feedback_enabled for orgs: #{ids}") ::Org.where(feedback_enabled: true, feedback_email_msg: "") .update_all(feedback_email_msg: DEFAULT_MSG) end diff --git a/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb b/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb index e190b22..ac637f9 100644 --- a/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb +++ b/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb @@ -6,12 +6,14 @@ DEFAULT_SUBJECT = "%{application_name}: Your plan has been submitted for feedback" def description - "Fix orgs where feedback_enabled is true" + "Fix orgs where feedback_email_subject is blank" end def call + ids = ::Org.where(feedback_enabled: true, feedback_email_subject: "").pluck(:id) + log("Adding default feedback_email_subject for orgs: #{ids}") ::Org.where(feedback_enabled: true, feedback_email_subject: "") - .update_all(feedback_email_subject: DEFAULT_SUBJECT) + .update_all(feedback_email_subject: DEFAULT_SUBJECT) end end end diff --git a/lib/data_cleanup/rules/org/fix_blank_is_other.rb b/lib/data_cleanup/rules/org/fix_blank_is_other.rb deleted file mode 100644 index b255720..0000000 --- a/lib/data_cleanup/rules/org/fix_blank_is_other.rb +++ /dev/null @@ -1,16 +0,0 @@ -module DataCleanup - module Rules - module Org - class FixBlankIsOther < Rules::Base - - def description - "Fix nil values for is_other on Org" - end - - def call - ::Org.where(is_other: nil).update_all(is_other: false) - end - end - end - end -end diff --git a/lib/data_cleanup/rules/org/fix_blank_language.rb b/lib/data_cleanup/rules/org/fix_blank_language.rb index 14ba0ff..b738e8a 100644 --- a/lib/data_cleanup/rules/org/fix_blank_language.rb +++ b/lib/data_cleanup/rules/org/fix_blank_language.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module DataCleanup module Rules module Org @@ -10,6 +12,8 @@ end def call + ids = ::Org.where(language: nil).pluck(:id) + log("Setting language to #{DEFAULT_LANGUAGE} for Orgs: #{ids}") ::Org.where(language: nil).update_all(language_id: DEFAULT_LANGUAGE.id) end end diff --git a/lib/data_cleanup/rules/org/fix_invalid_email.rb b/lib/data_cleanup/rules/org/fix_invalid_email.rb new file mode 100644 index 0000000..d5498a8 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_invalid_email.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix invalid email on Org + module Org + class FixInvalidEmail < Rules::Base + + def description + "Fix invalid email on Org" + end + + def call + orgs_with_contact_email = ::Org.all.select(&:contact_email?) + orgs_with_invalid_contact_email = orgs_with_contact_email.select do |org| + validator = EmailValidator.new(allow_nil: true, attributes: :contact_email) + validator.validate_each(org, :contact_email, org.contact_email) + end + + orgs_with_invalid_contact_email.each do |org| + log("Removing contact email from Org##{org.id}") + org.contact_email = nil + org.save(validate: false) + end + + ::Org.where(contact_email: "").update_all(contact_email: nil) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_name_with_spaces.rb b/lib/data_cleanup/rules/org/fix_name_with_spaces.rb new file mode 100644 index 0000000..5c6bfc7 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_name_with_spaces.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix name with space on Org + module Org + class FixNameWithSpace < Rules::Base + + def description + "Fix name with leading or trailing space on Org" + end + + def call + ::Org.update_all("name = TRIM(name)") + end + end + end + end +end diff --git a/lib/data_cleanup/rules/phase/fix_duplicate_number.rb b/lib/data_cleanup/rules/phase/fix_duplicate_number.rb index e733dcd..a35b1dc 100644 --- a/lib/data_cleanup/rules/phase/fix_duplicate_number.rb +++ b/lib/data_cleanup/rules/phase/fix_duplicate_number.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module DataCleanup module Rules # Fix duplicate number on Phase @@ -16,9 +17,10 @@ data.each do |values, count| number, template_id = *values ids = ::Phase.where(template_id: template_id) - .order("number ASC, created_at ASC") + .order("number, id") .pluck(:id) template = ::Template.find(template_id) + log("Reordering Phase number within Template##{template.id}") ::Phase.update_numbers!(*ids, parent: template) end end diff --git a/lib/data_cleanup/rules/plan/fix_blank_title.rb b/lib/data_cleanup/rules/plan/fix_blank_title.rb index 8dc87a9..0c1f594 100644 --- a/lib/data_cleanup/rules/plan/fix_blank_title.rb +++ b/lib/data_cleanup/rules/plan/fix_blank_title.rb @@ -8,7 +8,9 @@ end def call - ::Plan.where(title: [nil, '']).each do |plan| + ids = ::Plan.where(title: [nil, '']).ids + ::Plan.find(ids).each do |plan| + log("Adding default title to Plan##{plan.id}") plan.update(title: "My plan (#{plan.template.title})") end end diff --git a/lib/data_cleanup/rules/question/fix_duplicate_number.rb b/lib/data_cleanup/rules/question/fix_duplicate_number.rb index 083b8fb..65fb88d 100644 --- a/lib/data_cleanup/rules/question/fix_duplicate_number.rb +++ b/lib/data_cleanup/rules/question/fix_duplicate_number.rb @@ -19,6 +19,7 @@ .order("number ASC, created_at ASC") .pluck(:id) section = ::Section.find(section_id) + log("Reordering Question number in Section##{section.id}") ::Question.update_numbers!(*ids, parent: section) end end diff --git a/lib/data_cleanup/rules/question_format/fix_blank_description.rb b/lib/data_cleanup/rules/question_format/fix_blank_description.rb index 7c8f06c..cda21e1 100644 --- a/lib/data_cleanup/rules/question_format/fix_blank_description.rb +++ b/lib/data_cleanup/rules/question_format/fix_blank_description.rb @@ -11,6 +11,7 @@ def call ::QuestionFormat.where(description: "").each do |qf| + log("Adding default description to QuestionFormat##{qf.id}") qf.update!(description: "#{qf.title} format") end end diff --git a/lib/data_cleanup/rules/region/fix_blank_description.rb b/lib/data_cleanup/rules/region/fix_blank_description.rb index 1ddee40..b5389fa 100644 --- a/lib/data_cleanup/rules/region/fix_blank_description.rb +++ b/lib/data_cleanup/rules/region/fix_blank_description.rb @@ -9,6 +9,7 @@ def call ::Region.where(description: [nil, '']).each do |region| + log("Adding default description to Region##{region.id}") region.update!(description: "#{region.name} region") end end diff --git a/lib/data_cleanup/rules/role/fix_blank_plan.rb b/lib/data_cleanup/rules/role/fix_blank_plan.rb index 3e4219b..757b084 100644 --- a/lib/data_cleanup/rules/role/fix_blank_plan.rb +++ b/lib/data_cleanup/rules/role/fix_blank_plan.rb @@ -10,7 +10,9 @@ end def call - ::Role.where(plan: nil).destroy_all + ids = ::Role.where(plan: nil).ids + log("Destroying Roles without Plan: #{ids}") + ::Role.destroy(ids) end end end diff --git a/lib/data_cleanup/rules/section/fix_duplicate_number.rb b/lib/data_cleanup/rules/section/fix_duplicate_number.rb index c2c41c8..aa58c85 100644 --- a/lib/data_cleanup/rules/section/fix_duplicate_number.rb +++ b/lib/data_cleanup/rules/section/fix_duplicate_number.rb @@ -10,18 +10,74 @@ end def call - data = ::Section.group(:number, :phase_id) - .count - .select { |k,v| v > 1 } - data.each do |values, count| - number, phase_id = *values - ids = ::Section.where(phase_id: phase_id) - .order("number ASC, created_at ASC") - .pluck(:id) - phase = ::Phase.find(phase_id) + # A set of unique phase_ids that contain Sections with duplicate numbers + phase_ids = ::Section.group(:number, :phase_id) + .count + .select { |k,v| v > 1 } + .collect { |values, count| values.last } + .uniq + + phase_ids.each do |phase_id| + log("Re-setting number order for Sections in Phase##{phase_id}") + phase = ::Phase.find(phase_id) + sections = phase.sections + + # When every section within a Phase is modifiable, or when none is... + if sections.all?(&:modifiable?) || sections.all?(&:template?) + update_homogenous_set(phase) + + # When some of the sections are modifiable then... + else + update_heterogenous_set(phase) + end + end + end + + private + + # Re-sort the Phase's Sections by number, then by the ID + def update_homogenous_set(phase) + ids = phase.sections.order("number ASC, id ASC").ids + ::Section.update_numbers!(*ids, parent: phase) + end + + def update_heterogenous_set(phase) + # Array of duplicate Section numbers within this Phase + numbers = phase.sections + .group(:number) + .count + .select { |number, count| count > 1 } + .collect(&:first) + + # If there are duplicates in the #1 position + if numbers.include?(1) + # There should only be, if any, one prefixed modifiable Section + prefix = phase.sections.modifiable.where(number: 1).limit(1).ids + + # In the off-chance that there is more than one prefix Section, stick them + # after the unmodifiable block + erratic = phase.sections.modifiable.where(number: 1).offset(1).ids + + # Collect the unmodifiable Section ids in the order the should be displayed + unmodifiable = phase.sections.not_modifiable.order('number, id').ids + + # Then any additional Sections that come after the main block... + modifiable = phase.sections.modifiable.order('number, id').ids + + # Create one Array with all of the ids in the correct order. + ids = prefix + erratic + unmodifiable + modifiable + ::Section.update_numbers!(*ids, parent: phase) + + else + # Sort ids based on the number order. If there are duplicates, then take the + # earliest first. Unmodifiable sections from the template should be grouped + # together before the additional, modifiable sections are appended afterwards. + ids = phase.sections.not_modifiable.order("number, id").ids + ids += phase.sections.modifiable.order("number, id").ids ::Section.update_numbers!(*ids, parent: phase) end end + end end end diff --git a/lib/data_cleanup/rules/section/fix_nil_published.rb b/lib/data_cleanup/rules/section/fix_nil_published.rb deleted file mode 100644 index cb75525..0000000 --- a/lib/data_cleanup/rules/section/fix_nil_published.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true -module DataCleanup - module Rules - # Fix nil published on Section - module Section - class FixNilPublished < Rules::Base - - def description - "Fix nil published on Section" - end - - def call - ::Section.where(published: nil).update_all(published: false) - end - end - end - end -end diff --git a/lib/data_cleanup/rules/template/fix_blank_locale.rb b/lib/data_cleanup/rules/template/fix_blank_locale.rb index 4fdd5e7..786a7be 100644 --- a/lib/data_cleanup/rules/template/fix_blank_locale.rb +++ b/lib/data_cleanup/rules/template/fix_blank_locale.rb @@ -8,8 +8,9 @@ end def call - ::Template.where(locale: nil) - .update_all(locale: FastGettext.default_locale) + ids = ::Template.where(locale: nil).ids + log("Setting locale to #{FastGettext.default_locale} for Templates #{ids}") + ::Template.where(id: ids).update_all(locale: FastGettext.default_locale) end end end diff --git a/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb b/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb index 9676652..f012258 100644 --- a/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb +++ b/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb @@ -10,7 +10,9 @@ end def call - ::UserIdentifier.where(user: nil).destroy_all + ids = ::UserIdentifier.where(user: nil).ids + log("Destroying UserIdentifier where ids: #{ids} (no user)") + ::UserIdentifier.destroy(ids) end end end diff --git a/lib/tasks/data_cleanup.rake b/lib/tasks/data_cleanup.rake new file mode 100644 index 0000000..f84dfc0 --- /dev/null +++ b/lib/tasks/data_cleanup.rake @@ -0,0 +1,50 @@ +require "data_cleanup" + +namespace :data_cleanup do + + desc "Check each record on the DB is valid and report" + task :find_invalid_records => :environment do + DataCleanup.logger.info("\n== Finding invalid records =======================\n") + models.each do |model| + DataCleanup::ModelCheck.new(model).call + end + DataCleanup::Reporting.prepare! + DataCleanup::Reporting.report + end + + desc "Clean invalid records on the database" + task :clean_invalid_records => :environment do + DataCleanup.logger.info("\n== Cleaning invalid records =======================\n") + Dir[rule_paths].each do |rule_path| + load rule_path + klass_name = rule_path.split("rules/").last.gsub(".rb", '').classify + model_name = klass_name.split("::").first + opt, models = ARGV[1].to_s.split("=") + if opt.present? && opt =='INCLUDE' + next unless model_name.in?(models.split(",")) + elsif opt.present? && opt =='EXCLUDE' + next if model_name.in?(models.split(",")) + elsif opt.blank? + # :noop: + else + raise ArgumentError, "Unknown option: #{opt}" + end + rule_class = DataCleanup::Rules.const_get(klass_name) + rule = rule_class.new + puts rule.description + rule.call + end + end + + private + + def rule_paths + @rule_paths ||= Rails.root.join("lib", "data_cleanup", "rules", "*", "*.rb") + end + + def models + Dir[Rails.root.join("app", "models", "*.rb")].map do |model_path| + model_path.split("/").last.gsub(".rb", "").classify.constantize + end.sort_by(&:name) + end +end diff --git a/lib/tasks/heal_invalid_records.rake b/lib/tasks/heal_invalid_records.rake deleted file mode 100644 index 18a017f..0000000 --- a/lib/tasks/heal_invalid_records.rake +++ /dev/null @@ -1,47 +0,0 @@ -require "data_cleanup" - -namespace :data_cleanup do - - desc "Check each record on the DB is valid and report" - task :find_invalid_records => :environment do - models.each do |model| - DataCleanup::ModelCheck.new(model).call - end - DataCleanup::Reporting.prepare! - DataCleanup::Reporting.report - end - - desc "Clean invalid records on the database" - task :clean_invalid_records => :environment do - Dir[rule_paths].each do |rule_path| - load rule_path - klass_name = rule_path.split("rules/").last.gsub(".rb", '').classify - model_name = klass_name.split("::").first - opt, models = ARGV[1].to_s.split("=") - case opt - when 'INCLUDE' - next unless model_name.in?(models.split(",")) - when 'EXCLUDE' - next if model_name.in?(models.split(",")) - else - raise ArgumentError, "Unknown option: #{opt}" - end - rule_class = DataCleanup::Rules.const_get(klass_name) - rule = rule_class.new - puts rule.description - rule.call - end - end - - private - - def rule_paths - @rule_paths ||= Rails.root.join("lib", "data_cleanup", "rules", "*", "*.rb") - end - - def models - Dir[Rails.root.join("app", "models", "*.rb")].map do |model_path| - model_path.split("/").last.gsub(".rb", "").classify.constantize - end.sort_by(&:name) - end -end diff --git a/lib/tasks/upgrade.rake b/lib/tasks/upgrade.rake index 8a10e0c..9aec1be 100644 --- a/lib/tasks/upgrade.rake +++ b/lib/tasks/upgrade.rake @@ -1,14 +1,6 @@ require 'set' namespace :upgrade do - desc "Ensure all section numbers are unique per Phase." - task clean_section_numbers: :environment do - Phase.all.each do |phase| - ids_in_order = phase.sections.order("number, created_at").pluck(:id) - Section.update_numbers!(*ids_in_order, parent: phase) - end - end - desc "Upgrade to v1.1.2" task v1_1_2: :environment do Rake::Task['upgrade:check_org_contact_emails'].execute