-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update dataset #241
Update dataset #241
Conversation
(vec) | ||
(keep-indexed #(when (= id (:id %2)) %1)) | ||
(first))] | ||
(assoc (vec coll) index replacement))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a map
or a reduce
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And probably implement as (update-by-id coll (:id replacement) constantly replacement)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 7acb588
(vec) | ||
(keep-indexed #(when (= id (:id %2)) %1)) | ||
(first))] | ||
(apply update (vec coll) index update-fn args))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, use a map
or reduce
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 7acb588
Methods for bulk insert first check if the facility and facility types exist, and insert/update as needed. Insert facilities now returns a hash instead of a vector, indicating the status for each of the facilities requested to be inserted. Deletion methods now also accept an except key, to delete all facilities or types except those just upserted.
Add keys process-count and process-ids to import job to keep track of facilities to be processed, as only the new or moved facilities need reprocessing. Add new fsm states for deleting old facilities and types.
03c990f
to
c9e82f7
Compare
(map (fn [facility] | ||
(if-let [{id :id, :as existing} (some-> (filter #(= (:site-id %) (:site-id facility)) | ||
existing-facilities) | ||
(first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use reduce
similar to https://github.com/instedd/planwise/blob/master/src/planwise/client/utils.cljs#L84
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0693beb
[:moved id]))) | ||
; Insert the new facility otherwise | ||
(let [result (insert-facility! tx (assoc facility :dataset-id dataset-id))] | ||
[:new (:id result)])))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A cond
instead of the nested if
s would be more readable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0693beb
(let [current-types (select-types-in-dataset tx {:dataset-id dataset-id})] | ||
(->> types | ||
(mapv (fn [type] | ||
(if-let [existing-type (first (filter #(= (:name type) (:name %)) current-types))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before, implement a find-by
somewhere and use here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0693beb
@@ -42,7 +42,7 @@ | |||
|
|||
(defn- server-status->string | |||
[server-status] | |||
(case (:status server-status) | |||
(case (keyword (:status server-status)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed the labels for the new server statuses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4fcb97a
Support running a reimport from resmap for a dataset. Sites and types insertion is now an upsert, and inserts/updates records as needed. New steps are added to the import job FSM to delete stale types and facilities.
An Update button has been added to the datasets list as well, to trigger an update operation.
Fixes #180