Skip to content

Commit

Permalink
fix: Don't double capacity on data returned in providers CSV (#729)
Browse files Browse the repository at this point in the history 
We're using `merge-providers` to construct the final data to export, which
gathers providers and their attributes from several sources: providers set,
scenario changeset and scenario providers data. Since `merge-providers` adds up
its numeric fields, we were adding the capacity both from the final value found
in `:providers-data` and the original provider set and/or the changeset.
  • Loading branch information
@ggiraldez
ggiraldez authored Apr 30, 2024
1 parent 3e3ee6c commit 934f9a3
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/planwise/component/engine.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
[planwise.util.geo :as geo]
[planwise.util.files :as files]
[planwise.util.numbers :refer [abs float=]]
[planwise.model.providers :refer [merge-providers merge-provider]]
[planwise.model.providers :refer [merge-providers]]
[planwise.util.collections :refer [sum-by merge-collections-by]]
[integrant.core :as ig]
[clojure.java.io :as io]
Expand Down
9 changes: 1 addition & 8 deletions src/planwise/component/providers_set.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
[hugsql.core :as hugsql]
[clojure.edn :as edn]
[planwise.util.files :as files]
[planwise.util.collections :refer [csv-data->maps]]
[clojure.string :as str]
[clojure.set :as set]))

Expand All @@ -23,14 +24,6 @@
[store]
(get-in store [:db :spec]))

(defn- csv-data->maps
[csv-data]
(map zipmap
(->> (first csv-data)
(map keyword)
repeat)
(rest csv-data)))

(defn- import-provider
[store provider-set-id version csv-provider-data]
(try
Expand Down
11 changes: 9 additions & 2 deletions src/planwise/component/scenarios.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,16 @@
provider-set-id
(:provider-set-version project)
filter-options)
disabled-providers (map #(assoc % :capacity 0) disabled-providers)
new-providers (new-providers-to-export (:changeset scenario))
fields [:id :type :name :lat :lon :tags :capacity :required-capacity :used-capacity :satisfied-demand :unsatisfied-demand]]
;; final capacity for the scenario is stored in the scenario's :providers-data
;; so we remove it from the "base" collections to avoid adding them up
;; (and duplicating it) by merge-providers
disabled-providers (map #(dissoc % :capacity) disabled-providers)
providers (map #(dissoc % :capacity) providers)
new-providers (map #(dissoc % :capacity) new-providers)
fields [:id :type :name :lat :lon :tags
:capacity :required-capacity :used-capacity
:satisfied-demand :unsatisfied-demand]]
(map->csv
(merge-providers providers disabled-providers new-providers (:providers-data scenario))
fields)))
Expand Down
10 changes: 10 additions & 0 deletions src/planwise/util/collections.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,13 @@
(defn map-vals
[f m]
(reduce-kv (fn [acc k v] (assoc acc k (f v))) {} m))

(defn csv-data->maps
"Converts the result of csv/read-csv (ie. a collection of vectors of strings)
into a collection of maps using the first row (converted to keywords) as keys"
[csv-data]
(map zipmap
(->> (first csv-data)
(map keyword)
repeat)
(rest csv-data)))
78 changes: 69 additions & 9 deletions test/planwise/component/scenarios_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
[planwise.boundary.jobrunner :as jobrunner]
[planwise.model.scenarios :as model]
[planwise.test-system :as test-system]
[planwise.util.collections :refer [find-by]]
[planwise.test-utils :as utils]
[planwise.util.collections :refer [find-by csv-data->maps]]
[clj-time.core :as time]
[integrant.core :as ig]
[clojure.spec.alpha :as s]
[planwise.boundary.projects2 :as projects2]))
[clojure.data.csv :as csv]
[clojure.spec.alpha :as s])
(:import [org.postgis PGgeometry]))

(def owner-id 10)
(def project-id 20)
Expand All @@ -38,17 +40,58 @@
(def best-scenario-id 33)
(def other-scenario-id 34)

(def provider-set-id 40)
(def region-id 50)
(def provider-1 60)
(def provider-2 61)

(def fixture-with-scenarios
[[:users
[{:id owner-id :email "[email protected]"}]]
[:regions
[{:id region-id :country "Testiland" :name "Testiland" :the_geom (utils/sample-polygon :large)}]]
[:providers_set
[{:id provider-set-id :name "Joe's providers" :owner-id owner-id :last-version 1}]]
[:providers
[{:id provider-1 :source-id 1 :version 1 :provider-set-id provider-set-id
:name "Site A" :lat 0.0 :lon 0.0 :the_geom (utils/make-point 0.0 0.0) :capacity 10 :tags "tag1"}
{:id provider-2 :source-id 2 :version 1 :provider-set-id provider-set-id
:name "Site B" :lat 1.0 :lon 1.0 :the_geom (utils/make-point 1.0 1.0) :capacity 20 :tags "tag2"}]]
[:projects2
[{:id project-id :owner-id owner-id :name "" :config nil :provider-set-id nil}]]
[{:id project-id :owner-id owner-id :name "" :config nil
:region-id region-id
:provider-set-id provider-set-id :provider-set-version 1}]]
[:scenarios
[{:effort 0 :demand-coverage 100 :id initial-scenario-id :label "initial" :name "Initial" :project-id project-id :changeset "[]"}
{:effort 500 :demand-coverage 120 :id sub-optimal-scenario-id :label nil :name "S1" :project-id project-id :changeset "[]"}
{:effort 500 :demand-coverage 150 :id best-scenario-id :label nil :name "S2" :project-id project-id :changeset "[]"}
{:effort 1000 :demand-coverage 150 :id optimal-scenario-id :label nil :name "S3" :project-id project-id :changeset "[]"}
{:effort 1000 :demand-coverage 120 :id other-scenario-id :label nil :name "S4" :project-id project-id :changeset "[]"}]]])
{:id other-scenario-id
:project-id project-id
:name "S4"
:effort 1000
:demand-coverage 120
:label nil
:changeset (pr-str [{:action "create-provider" :id "new-0" :capacity 30 :location {:lat 0.5 :long 0.5}}
{:action "increase-provider" :id 61 :capacity 5}])
:providers-data (pr-str [{:id 60
:capacity 10
:required-capacity 10
:used-capacity 10
:satisfied-demand 100
:unsatisfied-demand 0}
{:id 61
:capacity 25
:required-capacity 30
:used-capacity 25
:satisfied-demand 250
:unsatisfied-demand 50}
{:id "new-0"
:capacity 30
:required-capacity 15
:used-capacity 15
:satisfied-demand 150
:unsatisfied-demand 0}])}]]])

(defn test-config
([]
Expand All @@ -57,11 +100,12 @@
(test-system/config
{:planwise.test/fixtures {:fixtures data}
:planwise.component/providers-set {:db (ig/ref :duct.database/sql)}
:planwise.component/projects2 {:db (ig/ref :duct.database/sql)
:planwise.component/projects2 {:db (ig/ref :duct.database/sql)
:providers-set (ig/ref :planwise.component/providers-set)}
:planwise.component/scenarios {:db (ig/ref :duct.database/sql)
:jobrunner (stub jobrunner/JobRunner
{:queue-job :enqueued})}})))
:planwise.component/scenarios {:db (ig/ref :duct.database/sql)
:providers-set (ig/ref :planwise.component/providers-set)
:jobrunner (stub jobrunner/JobRunner
{:queue-job :enqueued})}})))

;; ----------------------------------------------------------------------
;; Testing scenario's creation
Expand Down Expand Up @@ -187,3 +231,19 @@
initial-scenario (scenarios/get-scenario store initial-scenario-id)]
(is (nil? scenario))
(is (map? initial-scenario))))))

(deftest export-providers-data
(test-system/with-system (test-config fixture-with-scenarios)
(let [store (:planwise.component/scenarios system)
projects2 (:planwise.component/projects2 system)
project (projects2/get-project projects2 project-id)
scenario (scenarios/get-scenario store other-scenario-id)
csv (scenarios/export-providers-data store project scenario)
data (csv-data->maps (csv/read-csv csv))]
(is (= (set (keys (first data)))
#{:id :type :name :lat :lon :tags :capacity :required-capacity :used-capacity :satisfied-demand :unsatisfied-demand}))
(is (= 3 (count data)))
(letfn [(find-and-select [id] (-> data (find-by :id id) (select-keys [:id :capacity])))]
(is (= (find-and-select "60") {:id "60" :capacity "10"}))
(is (= (find-and-select "61") {:id "61" :capacity "25"}))
(is (= (find-and-select "new-0") {:id "new-0" :capacity "30"}))))))

0 comments on commit 934f9a3

Please sign in to comment.