Merge branch 'master' into feature/15-parameters

dlouise64 · web-flow · commit 8f5c4ea3b1b5 · 2019-05-15T16:27:30.000+12:00
diff --git a/README.md b/README.md
@@ -3,7 +3,7 @@
 [![Maintainability](https://api.codeclimate.com/v1/badges/580a9cc169de1c21c180/maintainability)](https://codeclimate.com/github/ServiceInnovationLab/RapuTure/maintainability)
 [![AwesomeCode Status for ServiceInnovationLab/RapuTure](https://awesomecode.io/projects/26aa53d4-bece-44c7-81d7-6c2f7648adec/status)](https://awesomecode.io/repos/ServiceInnovationLab/RapuTure)
 [![Build Status](https://travis-ci.org/ServiceInnovationLab/RapuTure.svg?branch=master)](https://travis-ci.org/ServiceInnovationLab/RapuTure)
-
+[![Test Coverage](https://api.codeclimate.com/v1/badges/580a9cc169de1c21c180/test_coverage)](https://codeclimate.com/github/ServiceInnovationLab/RapuTure/test_coverage)
 ## Overview
 
 Rapu Ture is a phrase that means "Exploring the rules/law"
diff --git a/app/controllers/scenarios_controller.rb b/app/controllers/scenarios_controller.rb
@@ -2,7 +2,12 @@
 
 class ScenariosController < ApplicationController
   def index
-    @scenarios = Scenario.all.order(:name)
+    if params[:variable_id].present?
+      @variable = Variable.find_by(name: params[:variable_id])
+      @scenarios = @variable.output_scenarios
+    else
+      @scenarios = Scenario.all.order(:name)
+    end
   end
 
   def show
diff --git a/app/models/scenario.rb b/app/models/scenario.rb
@@ -23,16 +23,43 @@ def other_input_keys
     keys - %w[persons families titled_properties]
   end
 
-  def parse_variables!
-    input_keys = get_all_keys(inputs)
-    output_keys = get_all_keys(outputs)
-    input_output_keys = input_keys + output_keys
+  def input_variables
+    variables.merge(ScenarioVariable.input)
+  end
 
-    self.variables = Variable.where(name: input_output_keys)
+  def output_variables
+    variables.merge(ScenarioVariable.output)
+  end
+
+  def parse_variables!
+    Scenario.transaction do
+      variables = []
+      %w[input output].each do |direction|
+        save_variable_links(direction).each do |v|
+          variables << v
+        end
+      end
+      # remove associating with any variables we no longer refer to
+      scenario_variables.where.not(variable: variables).destroy_all
+    end
   end
 
   private
 
+  def save_variable_links(direction)
+    hash_keys = if direction == 'input'
+                  get_all_keys(inputs)
+                elsif direction == 'output'
+                  get_all_keys(outputs)
+                else
+                  raise format('invalid direction %<direction>', direction: direction)
+                end
+    variables = Variable.where(name: hash_keys)
+    variables.each do |v|
+      ScenarioVariable.find_or_create_by! scenario: self, variable: v, direction: direction
+    end
+  end
+
   # This could use some refinement
   # Currently it is returning all the keys
   # Can't just user lower level though because of data structures like:
diff --git a/app/models/scenario_variable.rb b/app/models/scenario_variable.rb
@@ -3,4 +3,9 @@
 class ScenarioVariable < ApplicationRecord
   belongs_to :scenario
   belongs_to :variable
+  validates :direction, presence: true, inclusion: {
+    in: %w[input output], message: '%<value>s is not a valid direction'
+  }
+  scope :input, -> { where(direction: 'input') }
+  scope :output, -> { where(direction: 'output') }
 end
diff --git a/app/models/variable.rb b/app/models/variable.rb
@@ -13,6 +13,14 @@ class Variable < ApplicationRecord
   has_many :scenario_variables, dependent: :destroy
   has_many :scenarios, through: :scenario_variables
 
+  def input_scenarios
+    scenarios.merge(ScenarioVariable.input)
+  end
+
+  def output_scenarios
+    scenarios.merge(ScenarioVariable.output)
+  end
+
   has_and_belongs_to_many(:variables,
                           class_name: 'Variable',
                           join_table: 'links',
diff --git a/app/views/scenarios/_outputs.html.haml b/app/views/scenarios/_outputs.html.haml
@@ -18,7 +18,20 @@
           - value.each do |key, item|
             %li.list-group-item
               %strong= key
-              .pull-right= item.to_json
+              - if item.class == Array
+                %table.pull-right
+                  - item.each_with_index.map do |array_item, i|
+                    %tr
+                      %th{ scope: 'row' }= scenario.input_persons.keys[i]
+                      %td= array_item.to_json
+              - elsif item.is_a? Enumerable
+                %table.pull-right
+                  - item.each do |hash_key, hash_item|
+                    %tr
+                      %th= link_if_variable(hash_key)
+                      %td= hash_item.to_json
+              - else
+                .pull-right= item.to_json
     - else
       .card-body
         = link_if_variable(name)
diff --git a/app/views/scenarios/index.html.haml b/app/views/scenarios/index.html.haml
@@ -1,4 +1,8 @@
 .container
+  - if @variable.present?
+    .text-center
+      %h2.text-center Scenarios calculating #{link_to @variable, @variable}
+      %p= @variable.description
   .card-columns
     - @scenarios.each do |scenario|
       .card
diff --git a/app/views/scenarios/show.html.haml b/app/views/scenarios/show.html.haml
@@ -1,5 +1,12 @@
 .container
-  %h1.display-3= @scenario.name
+  %h1= @scenario.name
+  %p
+    %strong Period:
+    = @scenario.period
+  - if @scenario.error_margin.present?
+    %p
+      %strong Error Margin:
+      = @scenario.error_margin
   .row
     .col-md-6
       %h2 Inputs
diff --git a/app/views/variables/show.html.haml b/app/views/variables/show.html.haml
@@ -46,21 +46,28 @@
 
     - if @variable.has_formula?
       %hr/
-      %h3=t :formulas
-      %p
-        = t :formula_explanation, name: @variable.name
-        - @variable.spec['formulas'].each do |d, formula|
-          %h3= d
-          %p
-            = t(:formula_for_scenarios_from_date_onwards, formula_date: d)
-            = link_to 'More info on formulas', 'https://openfisca.org/doc/coding-the-legislation/10_basic_example.html'
-          %pre= display_formula formula['content']
+      .card
+        .card-header
+          %h3=t :formulas
+          %p= t :formula_explanation, name: @variable.name
+        .card-body
+          - @variable.spec['formulas'].each do |d, formula|
+            %h3= d
+            %p
+              = t(:formula_for_scenarios_from_date_onwards, formula_date: d)
+              = link_to 'More info on formulas', 'https://openfisca.org/doc/coding-the-legislation/10_basic_example.html'
+            %pre
+              %code= display_formula formula['content']
 
-    - if @variable.scenarios.size.positive?
+    - if @variable.output_scenarios.size.positive?
       %hr/
-      %h2 Scenarios (tests) for this variables
-      - @variable.scenarios.each do |scenario|
-        %li= link_to scenario.name, scenario
+      .card
+        .card-header
+          %h2 Scenarios calculating this variables
+        %ul.list-group.list-group-flush
+          - @variable.output_scenarios.each do |scenario|
+            %li.list-group-item
+              = link_to scenario.name, scenario
     %hr/
     %ul.nav
       - if @variable.references.present?
diff --git a/config/routes.rb b/config/routes.rb
@@ -3,7 +3,9 @@
 Rails.application.routes.draw do
   root to: 'entities#index'
   resources :entities, only: %i[show index]
-  resources :variables, only: %i[show index]
+  resources :variables, only: %i[show index] do
+    resources :scenarios, only: %i[index]
+  end
   resources :scenarios, only: %i[show index]
   resources :parameters, only: %i[show index]
   # get "/:page" => "pages#show"
diff --git a/db/migrate/20190513001329_scenario_variable_input_output.rb b/db/migrate/20190513001329_scenario_variable_input_output.rb
@@ -0,0 +1,6 @@
+class ScenarioVariableInputOutput < ActiveRecord::Migration[5.2]
+  def change
+    add_column :scenario_variables, :direction, :string, required: true
+    add_index :scenario_variables, [:scenario_id, :variable_id, :direction], unique: true, name: 'scenario_variables_key'
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -53,6 +53,8 @@
   create_table "scenario_variables", force: :cascade do |t|
     t.bigint "scenario_id"
     t.bigint "variable_id"
+    t.string "direction"
+    t.index ["scenario_id", "variable_id", "direction"], name: "scenario_variables_key", unique: true
     t.index ["scenario_id"], name: "index_scenario_variables_on_scenario_id"
     t.index ["variable_id"], name: "index_scenario_variables_on_variable_id"
   end
diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake
@@ -15,11 +15,13 @@ namespace :fetch do
   task entities: :environment do
     EntitiesFetchService.fetch_all
   end
+
   task parameters: :environment do
     ParametersFetchService.fetch_all do |p|
       puts p.name
     end
   end
+
   task scenarios: :environment do
     ScenariosFetchService.fetch_all
   end
diff --git a/spec/features/percy_spec.rb b/spec/features/percy_spec.rb
@@ -104,6 +104,8 @@
   it 'scenarios#index' do
     visit scenarios_path
     Percy.snapshot(page, name: 'scenarios#index')
+    visit variable_scenarios_path(variable_id: variable.name)
+    Percy.snapshot(page, name: 'variables/scenarios#index')
   end
 
   it 'scenarios#show' do
diff --git a/spec/features/scenarios/index_spec.rb b/spec/features/scenarios/index_spec.rb
@@ -10,7 +10,8 @@
                       name: 'a complicated situation',
                       inputs: {
                         'persons' => { 'fulltime_uni_student' => { 'age' => 21, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__is_enrolled_fulltime' => true, 'student_allowance__meets_attendance_and_performance_requirements' => true }, 'Parttime_student' => { 'age' => 18, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__approved_to_study_parttime' => true }, 'Overseas_student' => { 'age' => 27, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__approved_to_study_overseas' => true }, 'Refugee' => { 'age' => 25, 'immigration__is_recognised_refugee' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__is_enrolled_fulltime' => true, 'student_allowance__meets_attendance_and_performance_requirements' => true }, 'Not_a_student' => { 'age' => 50, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => false } }, 'families' => { 'Whanau' => { 'others' => %w[fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student] } }, 'titled_properties' => { 'whare' => { 'others' => %w[
-                          fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student] } }
+                          fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student
+                        ] } }
                       },
                       outputs: { 'student_allowance__eligible_for_basic_grant' => [
                         true, true, true, true, false
diff --git a/spec/features/scenarios/show_spec.rb b/spec/features/scenarios/show_spec.rb
@@ -6,12 +6,13 @@
   let(:input_hash) do
     {
       'persons' => { 'fulltime_uni_student' => { 'age' => 21, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__is_enrolled_fulltime' => true, 'student_allowance__meets_attendance_and_performance_requirements' => true }, 'Parttime_student' => { 'age' => 18, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__approved_to_study_parttime' => true }, 'Overseas_student' => { 'age' => 27, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__approved_to_study_overseas' => true }, 'Refugee' => { 'age' => 25, 'immigration__is_recognised_refugee' => true, 'student_allowance__is_tertiary_student' => true, 'student_allowance__is_enrolled_fulltime' => true, 'student_allowance__meets_attendance_and_performance_requirements' => true }, 'Not_a_student' => { 'age' => 50, 'is_nz_citizen' => true, 'social_security__is_ordinarily_resident_in_new_zealand' => true, 'student_allowance__is_tertiary_student' => false } }, 'families' => { 'Whanau' => { 'others' => %w[fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student] } }, 'titled_properties' => { 'whare' => { 'others' => %w[
-        fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student] } }
+        fulltime_uni_student Overseas_student Refugee Not_a_student Parttime_student
+      ] } }
     }
   end
   let(:output_hash) do
     { 'student_allowance__eligible_for_basic_grant' => [true, true, true,
-                                                       true, false] }
+                                                        true, false] }
   end
   let(:our_scenario) do
     FactoryBot.create :variable, name: 'age', description: 'age in years'
diff --git a/spec/models/scenario_spec.rb b/spec/models/scenario_spec.rb
@@ -24,24 +24,56 @@
         }
       }
     end
+    let(:output_hash) do
+      { 'student_allowance__eligible_for_basic_grant' => [true] }
+    end
     let(:expected_variables) do
+      expected_input_variables + expected_output_variables
+    end
+    let(:expected_input_variables) do
       %w[age is_nz_citizen social_security__is_ordinarily_resident_in_new_zealand
          student_allowance__is_tertiary_student student_allowance__is_enrolled_fulltime
          student_allowance__meets_attendance_and_performance_requirements]
     end
+    let(:expected_output_variables) do
+      ['student_allowance__eligible_for_basic_grant']
+    end
     let!(:complicated_scenario) do
       expected_variables.each do |variable_name|
         FactoryBot.create :variable, name: variable_name
       end
       FactoryBot.create :scenario,
                         name: 'a complicated situation',
                         inputs: input_hash,
-                        outputs: { 'student_allowance__eligible_for_basic_grant' => [true] },
+                        outputs: output_hash,
                         period: '2019-05',
                         error_margin: 1.0,
                         namespace: 'ghostchips'
     end
     before { complicated_scenario.parse_variables! }
     it { expect(complicated_scenario.variables.pluck(:name)).to eq expected_variables }
+    it { expect(complicated_scenario.input_variables.pluck(:name)).to eq expected_input_variables }
+    it { expect(complicated_scenario.output_variables.pluck(:name)).to eq expected_output_variables }
+    describe 'only allows input/output variables' do
+      it { expect { ScenarioVariable.create! variable: variable, scenario: scenario }.to raise_error }
+      it { expect { ScenarioVariable.create! variable: variable, scenario: scenario, direction: 'down' }.to raise_error }
+    end
+    describe 'no duplicates allowed' do
+      subject { complicated_scenario.output_variables.pluck(:name) }
+      scenario do
+        complicated_scenario.parse_variables!
+        expect(subject).to eq ['student_allowance__eligible_for_basic_grant']
+        complicated_scenario.parse_variables!
+        expect(subject).to eq ['student_allowance__eligible_for_basic_grant']
+      end
+    end
+    describe 'removes variables we no longer refer to' do
+      scenario do
+        FactoryBot.create :variable, name: 'hates_marshmallows'
+        complicated_scenario.update!(outputs: { "hates_marshmallows": [true] })
+        complicated_scenario.parse_variables!
+        expect(complicated_scenario.output_variables.pluck(:name)).to eq ['hates_marshmallows']
+      end
+    end
   end
 end
diff --git a/spec/models/variable_spec.rb b/spec/models/variable_spec.rb
@@ -10,10 +10,10 @@
   end
 
   describe 'links' do
-    let!(:parent) {
+    let!(:parent) do
       FactoryBot.create :variable, reversed_variables: [child1,
                                                         child2]
-    }
+    end
     let(:child1) { FactoryBot.create :variable }
     let(:child2) { FactoryBot.create :variable }
 
