Ensure bug with duplicate periods does not happen

lib/orbf/rules_engine/fetch_data/fetch_data_analytics.rb

@@ -12,10 +12,6 @@ def call
         return [] if analytics_activity_states.none?
 
         combined_response = Array(without_yearly_periods).each_slice(MAX_PERIODS_PER_FETCH).inject({}) do |result, period_slice|
-          puts({periods:            period_slice.join(";"),
-          organisation_units: orgunit_ext_ids,
-          data_elements:      data_elements}.to_json)
-
           analytics_response = dhis2_connection.analytics.list(
             periods:            period_slice.join(";"),
             organisation_units: orgunit_ext_ids,
@@ -75,7 +71,7 @@ def orgunit_ext_ids
       end
 
       def without_yearly_periods
-        package_arguments.map(&:periods).map { |p| p[0..-3] }.uniq.flatten
+        package_arguments.map(&:periods).map { |p| p[0..-3] }.flatten.uniq.sort
       end
     end
   end

lib/orbf/rules_engine/fetch_data/periods_resolver.rb

@@ -8,6 +8,7 @@ def initialize(package, invoice_period)
         @invoice_period = invoice_period
       end
 
+      # Will return an array of periods, guaranteed to be unique.
       def call
         from_values_span(package, invoice_period).merge(
           from_package_frequency(package, invoice_period)

spec/analytics_datavalues_spec.rb

@@ -85,6 +85,11 @@
                 build_activity_formula(
                   "de_m_from_indic", "real_indicator_value",
                   ""
+                ),
+
+                build_activity_formula(
+                  "de_m_average", "avg(%{real_indicator_value_last_6_months_window_values})",
+                  ""
                 )
               ]
             )
@@ -144,25 +149,39 @@
 
   it "builds problem per category option combo" do
     WebMock::Config.instance.query_values_notation = :flat_array
+    all = %w(202002 202003 202004 202005 202006 202007 202008 202009)
 
-    stub_request(:get, "https://sample/api/analytics?dimension=dx:dNaUIFe7vkY&dimension=ou:kH9XaP6eoDo&dimension=pe:202007%3B202008%3B202009%3B2020Q3")
-      .to_return(status: 200, body: JSON.pretty_generate(
-        {
-          "height": 3, "rows": [
-            ["dNaUIFe7vkY", "kH9XaP6eoDo", "202007", "2.0"],
-            ["dNaUIFe7vkY", "kH9XaP6eoDo", "202008", "2.0"],
-            ["dNaUIFe7vkY", "kH9XaP6eoDo", "2020Q3", "4.0"]
-          ], "width": 4
-        }
-      ))
+    [
+      all[0...5],
+      all[5..-1] + ["2020Q3"]
+    ].each do |periods|
+      pe = periods.join(";")
+      rows = periods.map do |period|
+        if "Q3" == period[0..-2]
+          value = "134"
+        else
+          value = period[-2..-1].to_i*100
+        end
+        ["dNaUIFe7vkY", "kH9XaP6eoDo", period, value]
+      end
+
+      stub_request(:get, "https://sample/api/analytics?dimension=dx:dNaUIFe7vkY&dimension=ou:kH9XaP6eoDo&dimension=pe:#{pe}")
+        .to_return(status: 200, body: JSON.pretty_generate(
+                     {
+                       "height": 3, "rows": rows, "width": 4
+                     }
+                   ))
+
+    end
 
-    stub_request(:get, "https://sample/api/dataValueSets?children=false&dataSet=ds1&dataSet=ds2&orgUnit=path&period=2020&period=202007&period=202008&period=202009&period=2020July&period=2020Q3")
+    pe = (all + %w(2020 2020July 2020Q3)).sort.map{|s| "period=#{s}" }.join("&")
+    stub_request(:get, "https://sample/api/dataValueSets?children=false&dataSet=ds1&dataSet=ds2&orgUnit=path&#{pe}")
       .to_return(status: 200, body: JSON.pretty_generate(
         { "dataValues" => [
-          { "dataElement": indicator_dataelement_id, period: "202008", value: "2.0" ,categoryCombo:"Vo4mFUa9rlC"},
-          { "dataElement": indicator_dataelement_id, period: "202008", value: "0.0" ,categoryCombo:"PQrXhDwCZBF"},
-          { "dataElement": indicator_dataelement_id, period: "202009", value: "2.0" ,categoryCombo:"Vo4mFUa9rlC"},
-          { "dataElement": indicator_dataelement_id, period: "202009", value: "0.0" ,categoryCombo:"PQrXhDwCZBF"}
+          { "dataElement": indicator_dataelement_id, period: "202008", value: "80.0" ,categoryCombo:"Vo4mFUa9rlC"},
+          { "dataElement": indicator_dataelement_id, period: "202008", value: "81.0" ,categoryCombo:"PQrXhDwCZBF"},
+          { "dataElement": indicator_dataelement_id, period: "202009", value: "90.0" ,categoryCombo:"Vo4mFUa9rlC"},
+          { "dataElement": indicator_dataelement_id, period: "202009", value: "91.0" ,categoryCombo:"PQrXhDwCZBF"}
         ] }
       ))
 

spec/lib/orbf/rules_engine/fetch_data/fetch_data_analytics_spec.rb

@@ -99,24 +99,81 @@
     WebMock::Config.instance.query_values_notation = nil
   end
 
-  it "splits out the API calls with too many different periods" do
-    periods = ["202004", "202005", "202006", "202007","202008", "202009", "202010", "202011"]
-    periods_with_years = periods.each_slice(4).map { |slice| slice += ["2020", "2019July"] }
+  describe '#without_yearly_periods' do
+    it "removes the yearly periods" do
+      periods = ["202004", "202005", "202006", "202007"]
+      yearly_periods = ["2020", "2019July"] # Added in PeriodResolver#from_package_frequency
+      periods_with_years = (periods + yearly_periods)
+
+      package_arguments = [
+        Orbf::RulesEngine::PackageArguments.with(
+          periods:          periods_with_years,
+          orgunits:         Orbf::RulesEngine::OrgUnits.new(
+            orgunits: [orgunit_1], package: package
+          ),
+          datasets_ext_ids: [],
+          package:          package
+        )
+      ]
+
+      fetcher = described_class.new(dhis2_connection, package_arguments)
+      expect(fetcher.send(:without_yearly_periods)).to eq(periods)
+    end
 
-    package_arguments = periods_with_years.map do |periods|
+    it "removes duplicate periods if any" do
+      periods = [
+        ["202004", "202005", "202006", "202007"],
+        ["202004",                              "202008", "202009", "202010"],
+        ["202004",                              "202008",           "202010"],
+      ]
+      periods_with_years = periods.map { |slice| slice += ["2020", "2019July"] }
+
+      # Create three PackageArguments with some overlap in between
+      package_arguments = periods_with_years.map do |periods|
+        Orbf::RulesEngine::PackageArguments.with(
+          periods:          periods,
+          orgunits:         Orbf::RulesEngine::OrgUnits.new(
+            orgunits: [orgunit_1], package: package
+          ),
+          datasets_ext_ids: [],
+          package:          package
+        )
+      end
+      fetcher = described_class.new(dhis2_connection, package_arguments)
+      # Only expect from 04 to 11, and all of them once
+      expected = (4..10).map {|i| "2020%02d" % i}
+      expect(fetcher.send(:without_yearly_periods)).to eq(expected)
+    end
+  end
+
+  it "splits out the API calls with too many different periods" do
+    max_periods = described_class::MAX_PERIODS_PER_FETCH
+    # Let's set up MAX_PERIODS_PER_FETCH+1 periods
+    periods = (0..(max_periods + 1)).map {|i| "2020%02d" % (i+1)}
+    yearly_periods = ["2020", "2019July"]
+    org_unit = Orbf::RulesEngine::OrgUnits.new(orgunits: [orgunit_1], package: package)
+
+    # Now create three packages, which each use a different subset of the periods
+    package_arguments = [
+      periods[0..2],
+      periods[2..4],
+      periods
+    ].map do |period_subset|
       Orbf::RulesEngine::PackageArguments.with(
-        periods:          periods,
-        orgunits:         Orbf::RulesEngine::OrgUnits.new(
-          orgunits: [orgunit_1], package: package
-        ),
+        periods:          period_subset + yearly_periods,
+        orgunits:         org_unit,
         datasets_ext_ids: [],
         package:          package
       )
     end
 
-    fetcher = described_class.new(dhis2_connection, package_arguments)
-
-    stubbed_requests = periods.each_slice(described_class::MAX_PERIODS_PER_FETCH).map do |period_slice|
+    # We know this will take two requests (MAX_PERIODS_PER_FETCH+1)
+    # First one will be from 0..MAX_PERIODS_PER_FETCH-1
+    # Second one will be the MAX_PERIODS_PER_FETCH til the end
+    stubbed_requests = [
+      periods[0...max_periods],
+      periods[max_periods..-1]
+    ].map do |period_slice|
       pe = period_slice.join(";")
       rows = period_slice.inject([]) do |result, period|
         result << ["dhis2_de_1", orgunit_1.ext_id, period, "1.4"]
@@ -125,17 +182,16 @@
         result
       end
       stub_request(:get, "https://play.dhis2.org/2.28/api/analytics?dimension=dx:dhis2_de_1%3Bdhis2_indic_1%3Bdhis2_de_1.coc_1&dimension=ou:1&dimension=pe:#{pe}")
-              .to_return(status: 200, body: JSON.pretty_generate(
-                "rows" => rows
-              ), headers: {})
+        .to_return(status: 200, body: JSON.pretty_generate(
+                     "rows" => rows
+                   ), headers: {})
     end
+
     fetcher = described_class.new(dhis2_connection, package_arguments)
     values = fetcher.call
     stubbed_requests.each {|sr| expect(sr).to have_been_made.once }
 
-
     periods.each do |period|
-
       expect(values).to include({ "attributeOptionCombo" => "default",
                                   "categoryOptionCombo"  => "default",
                                   "dataElement"          => "dhis2_de_1",

spec/lib/orbf/rules_engine/fetch_data/periods_resolver_spec.rb

@@ -20,6 +20,10 @@
             Orbf::RulesEngine::Formula.new(
               "increase",
               "safe_div(achieved,sum(%{achieved_previous_year_same_quarter_monthly_values}/4)"
+            ),
+            Orbf::RulesEngine::Formula.new(
+              "increase",
+              "safe_div(achieved,sum(%{achieved_last_12_months_window_values})"
             )
           ]
         )
@@ -52,8 +56,16 @@
   end
 
   it "resolve periods from package frequency and activity span _values" do
-    expect(described_class.new(quantity_package, "2016Q1").call).to eq(
-      %w[201501 201502 201503 201601 201602 201603 2016 2015July]
-    )
+    previous_year_same_quarter = [
+      "201501", "201502", "201503", "201504", "201505"
+    ]
+    last_12_months = [
+      "201603", "201602", "201601", "201512", "201511",
+      "201510", "201509", "201508", "201507", "201506",
+      "201505", "201504"
+    ]
+    yearlies = ["2016", "2015July"]
+    expected = (previous_year_same_quarter + last_12_months).uniq.sort + yearlies
+    expect(described_class.new(quantity_package, "2016Q1").call).to eq(expected)
   end
 end