portagenetwork / roadmap

Developed by the the Alliance in collaboration with University of Alberta, DMP Assistant a data management planning tool, forking the DMP Roadmap codebase
MIT License
6 stars 1 forks source link

Possible Unwanted Outcome From `rake export_production_data:seed_5_export` #699

Open aaronskiba opened 3 months ago

aaronskiba commented 3 months ago

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0) 4.0.2+portage-4.0.1

Description

Offenses:

lib/tasks/export_portage.rake:185:12: C: Performance/CollectionLiteralInLoop: Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
        if [20..50].include?(index)
           ^^^^^^^^
lib/tasks/export_portage.rake:187:15: C: Performance/CollectionLiteralInLoop: Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
        elsif [60..90].include?(index)
              ^^^^^^^^

Here is the corresponding code in lib/tasks/export_portage.rake:

      Plan.where(org_id: org_list).all.each_with_index do |plan, index|
        plan.title = "Test Plan #{index}"
        plan.description = Faker::Lorem.sentence
        # force a few plan to use modified template from the two test organizations for statistics
        if [20..50].include?(index)
          plan.template = Template.find(title: 'Portage Template-Test1')
        elsif [60..90].include?(index)
          plan.template = Template.find(title: 'Portage Template-Test2')
        end

The issue is that if [20..50].include?(index) and elsif [60..90].include?(index) will always be false:

3.0.5 :001 > [20..50].include?(30)
 => false 
3.0.5 :002 > [20..50].count
 => 1 
3.0.5 :003 > [20..50][0]
 => 20..50 
3.0.5 :004 > [20..50][0].include?(30)
 => true 

We will have to examine the consequences of this.

aaronskiba commented 3 months ago

Here is the full rake task:

# seed5: export all plan which org belongs to testers, this task generate the seed file that runs lastly
  desc 'Export plan content from 3.0.2 database to seeds_5.rb'
  task seed_5_export: :environment do
    file_name = 'db/seeds/sandbox/seeds_5.rb'
    FileUtils.rm_f(file_name)
    excluded_keys = %w[created_at updated_at start_date end_date]
    org_list = [Rails.application.secrets.funder_org_id.to_i, Rails.application.secrets.english_org_id.to_i,
                Rails.application.secrets.french_org_id.to_i]
    File.open(file_name, 'a') do |f|
      Plan.where(org_id: org_list).all.each_with_index do |plan, index|
        plan.title = "Test Plan #{index}"
        plan.description = Faker::Lorem.sentence
        # force a few plan to use modified template from the two test organizations for statistics
        if [20..50].include?(index)
          plan.template = Template.find(title: 'Portage Template-Test1')
        elsif [60..90].include?(index)
          plan.template = Template.find(title: 'Portage Template-Test2')
        end
        serialized = plan.serializable_hash.delete_if { |key, _value| excluded_keys.include?(key) }
        f.puts "Plan.create(#{serialized})"
        # import related roles
        Role.where(plan_id: plan.id).all.each do |role|
          role.user_id = if plan.org_id == Rails.application.secrets.funder_org_id.to_i # change all user id to 1
                           1
                         elsif plan.org_id == Rails.application.secrets.english_org_id.to_i # change all user id to 2
                           2
                         else # change all user id to 3
                           3
                         end
          serialized = role.serializable_hash.delete_if { |key, _value| excluded_keys.include?(key) }
          f.puts "Role.create(#{serialized})"
        end
      end
    end
  end
end