Skip to content
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.

Sign up for GitHub

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

Jump to bottom

OY-4699 API and functionality for updating all form koodisto values #1518

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

OY-4699 API and functionality for updating all form koodisto values #1518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d342c7
OY-4699 API and functionality for updating all form koodisto values
vaeinoe Mar 6, 2024
a3dea0e
OY-4699 sort codes according to Finnish labels when updating
vaeinoe Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions spec/ataru/fixtures/form.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,57 @@
:metadata metadata
:fieldType "attachment"
:label {:fi "Liite ilman vastausta"}}]})

(def form-with-koodisto-source
{:id 981230123
:name {:fi "Test fixture for options vs koodisto"}
:key "koodisto-test-form"
:created-by "1.2.246.562.11.11111111111"
:organization-oid "1.2.246.562.10.0439845"
:created-time "2016-07-28T09:58:34.217+03:00"
:locked nil
:locked-by nil
:content [{:id "kysymys_1",
:options [{:value "2"},
{:value "1"},
{:value "4"},
{:value "3"},
{:value "5"}],
:metadata metadata,
:fieldType "dropdown",
:fieldClass "formField",
:validators ["required"],
:koodisto-source {:uri "kktutkinnot",
:title "Kk-tutkinnot",
:version 1,
:allow-invalid? false}}]})

(def form-test-followup-value
{:value "3"
:label {:fi "" :sv ""}
:followups [(assoc (component/text-field metadata)
:id "text")]})

(def form-with-koodisto-source-and-followup
{:id 981230123
:name {:fi "Test fixture for options vs koodisto"}
:key "koodisto-test-form"
:created-by "1.2.246.562.11.11111111111"
:organization-oid "1.2.246.562.10.0439845"
:created-time "2016-07-28T09:58:34.217+03:00"
:locked nil
:locked-by nil
:content [{:id "kysymys_1",
:options [{:value "2" :label {:fi "" :sv ""}},
{:value "1" :label {:fi "" :sv ""}},
{:value "4" :label {:fi "" :sv ""}},
form-test-followup-value,
{:value "5" :label {:fi "" :sv ""}}],
:metadata metadata,
:fieldType "dropdown",
:fieldClass "formField",
:validators ["required"],
:koodisto-source {:uri "kktutkinnot",
:title "Kk-tutkinnot",
:version 1,
:allow-invalid? false}}]})
101 changes: 100 additions & 1 deletion spec/ataru/virkailija/virkailija_routes_spec.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@
[ataru.organization-service.organization-service :as org-service]
[ataru.person-service.person-service :as person-service]
[ataru.tarjonta-service.mock-tarjonta-service :as tarjonta-service]
[ataru.cache.cache-service :as cache-service]
[ataru.test-utils :refer [login should-have-header]]
[ataru.virkailija.editor.form-diff :as form-diff]
[ataru.virkailija.virkailija-routes :as v]
[ataru.forms.form-store :as form-store]
[ataru.koodisto.koodisto :as koodisto]
[cheshire.core :as json]
[clj-ring-db-session.session.session-store :refer [create-session-store]]
[ring.mock.request :as mock]
[speclj.core :refer [before describe it run-specs should should-not-be-nil should= tags with]]
[speclj.core :refer [before describe it run-specs should should-contain
should-be-nil should-not-be-nil should= tags with]]
[com.stuartsierra.component :as component]))

(def virkailija-routes
Expand All @@ -27,6 +31,11 @@
:kayttooikeus-service (kayttooikeus-service/->FakeKayttooikeusService)
:person-service (person-service/->FakePersonService)
:audit-logger (audit-log/new-dummy-audit-logger)
:koodisto-cache (reify cache-service/Cache
(get-from [_ _])
(get-many-from [_ _])
(remove-from [_ _])
(clear-all [_]))
:application-service (component/using
(application-service/new-application-service)
[:organization-service
Expand Down Expand Up @@ -90,6 +99,13 @@
(mock/content-type "application/json")
((deref virkailija-routes))))

(defn- refresh-form-codes [key]
(-> (mock/request :put (str "/lomake-editori/api/forms/" key "/refresh-codes"))
(update-in [:headers] assoc "cookie" (login @virkailija-routes))
(mock/content-type "application/json")
((deref virkailija-routes))
(update :body (comp (fn [content] (json/parse-string content true)) slurp))))

(declare resp)

(describe "GET /lomake-editori"
Expand Down Expand Up @@ -365,4 +381,87 @@
status (:status resp)]
(should= 200 status))))

(describe "Refreshing form code values"
(tags :unit :koodisto)

(defn dummy-koodisto-options [values]
(map (fn [val]
{:value val :label {:fi "" :sv ""}}) values))

(defn post-refresh-and-check [expected]
(let [resp (refresh-form-codes "koodisto-test-form")
status (:status resp)
id (get-in resp [:body :id])]
(should= expected status)
id))

(defn get-first-option-values [form]
(->> form
:content
first
:options
(map :value)
sort))

(it "Should update new codes on form based on new koodisto values"
(with-redefs [koodisto/get-koodisto-options (fn [_ uri _ _]
(case uri
"kktutkinnot"
(dummy-koodisto-options ["1" "2" "3" "4" "5" "6"])))]
(db/init-db-fixture fixtures/form-with-koodisto-source)
(let [id (post-refresh-and-check 200)
new-form (form-store/fetch-by-id id)
new-option-values (get-first-option-values new-form)]
(should= '("" "1" "2" "3" "4" "5" "6") new-option-values))))

(it "Should alphabetize based on Finnish labels"
(with-redefs [koodisto/get-koodisto-options (fn [_ uri _ _]
(case uri
"kktutkinnot"
[{:value 1 :label {:fi "Beekkonen" :sv ""}}
{:value 2 :label {:fi "Aakkonen" :sv ""}}
{:value 3 :label {:fi "Ceekkonen" :sv ""}}]))]
(db/init-db-fixture fixtures/form-with-koodisto-source)
(let [id (post-refresh-and-check 200)
new-form (form-store/fetch-by-id id)
new-options (->> new-form
:content
first
:options)]
(should= [{:label {:en "", :fi "", :sv ""}, :value ""}
{:value 2 :label {:fi "Aakkonen" :sv ""}}
{:value 1 :label {:fi "Beekkonen" :sv ""}}
{:value 3 :label {:fi "Ceekkonen" :sv ""}}] new-options))))

(it "Should delete existing codes when they do not have followups"
(with-redefs [koodisto/get-koodisto-options (fn [_ uri _ _]
(case uri
"kktutkinnot"
(dummy-koodisto-options ["1" "2" "5"])))]
(db/init-db-fixture fixtures/form-with-koodisto-source)
(let [id (post-refresh-and-check 200)
new-form (form-store/fetch-by-id id)
new-option-values (get-first-option-values new-form)]
(should= '("" "1" "2" "5") new-option-values))))

(it "Should keep existing followups while updating koodisto values"
(with-redefs [koodisto/get-koodisto-options (fn [_ uri _ _]
(case uri
"kktutkinnot"
(dummy-koodisto-options ["1" "2" "3" "4" "5"])))]
(db/init-db-fixture fixtures/form-with-koodisto-source-and-followup)
(let [id (post-refresh-and-check 200)
new-form (form-store/fetch-by-id id)
new-options (->> new-form :content first :options)]
(should-contain fixtures/form-test-followup-value new-options))))

(it "Should fail if there are followups in option values that would be silently removed"
(with-redefs [koodisto/get-koodisto-options (fn [_ uri _ _]
(case uri
"kktutkinnot"
(dummy-koodisto-options ["1" "2" "4" "5"])))]
(db/init-db-fixture fixtures/form-with-koodisto-source-and-followup)
(let [id (post-refresh-and-check 400)]
(should-be-nil id)))))

(run-specs)
17 changes: 16 additions & 1 deletion src/clj/ataru/forms/form_access_control.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
[ataru.organization-service.session-organizations :as session-orgs]
[ataru.middleware.user-feedback :refer [user-feedback-exception]]
[ataru.tarjonta.haku :as haku]
[ataru.koodisto.koodisto :as koodisto]
[taoensso.timbre :as log]))

(defn- form-allowed-by-id?
Expand Down Expand Up @@ -100,14 +101,18 @@
(defn post-form [form session tarjonta-service organization-service audit-logger]
(let [organization-oids (map :oid (get-organizations-with-edit-rights session))
first-org-oid (first organization-oids)
form-with-org (assoc form :organization-oid (or (:organization-oid form) first-org-oid))]
form-with-org (assoc form :organization-oid (or (:organization-oid form) first-org-oid))
key (:key form)]
(log/info "Checking form field ID duplicates with form key" key)
(check-form-field-id-duplicates form)
(log/info "Checking form edit authorization with form key" key)
(check-edit-authorization
form-with-org
session
tarjonta-service
organization-service
(fn []
(log/info "Creating or updating form with key" key)
(form-store/create-form-or-increment-version!
(assoc
form-with-org
Expand Down Expand Up @@ -150,6 +155,16 @@
updated-form (form-diff/apply-operations coerced-form operations)]
(post-form updated-form session tarjonta-service organization-service audit-logger)))))

(defn refresh-form-codes
[form-key session tarjonta-service organization-service koodisto-cache audit-logger]
(log/info "Requested to refresh codes for form, key" form-key)
(let [form (form-store/fetch-by-key form-key)
has-applications? (form-store/form-has-applications form-key)
updated-form (koodisto/populate-form-koodisto-fields koodisto-cache form true)]
(when has-applications? (throw (user-feedback-exception (str "Lomakkeella " (:key form) " on hakemuksia."))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tän vois varmaan tehdä jo ennen tota lettiä?

tarjonta-service:stä pitäisi saada haun tiedot ja löytyy myös koodia missä niitä hakuaikoja tarkastellaan, jos haluaa senkin implementoida tähän.

(log/info "Saving form with refreshed code values, key" form-key)
(post-form updated-form session tarjonta-service organization-service audit-logger)))

(defn delete-form [form-id session tarjonta-service organization-service audit-logger]
(let [form (form-store/fetch-latest-version form-id)]
(if (> (:application-count form) 0)
Expand Down
59 changes: 41 additions & 18 deletions src/clj/ataru/koodisto/koodisto.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
(ns ataru.koodisto.koodisto
(:require [ataru.util :as util]
[ataru.component-data.value-transformers :refer [update-options-while-keeping-existing-followups]]
[ataru.cache.cache-service :as cache-service])
[ataru.cache.cache-service :as cache-service]
[ataru.middleware.user-feedback :refer [user-feedback-exception]]
[taoensso.timbre :as log])
(:import java.time.ZonedDateTime))

(defn encode-koodisto-key [{:keys [uri version]}]
Expand Down Expand Up @@ -36,24 +38,42 @@
(= "attachment" (:fieldType field))
(-> field :params :fetch-info-from-kouta?)))

(defn- removed-followup-values
[newest-options existing-options remove-existing]
(if remove-existing
(let [new-values (set (map :value newest-options))
options-to-be-removed (filter #(not (contains? new-values (:value %))) existing-options)]
(filter #(not-empty (:followups %)) options-to-be-removed))
[]))

(defn- populate-choice-field-from-koodisto
[koodisto-cache field]
[koodisto-cache field remove-existing]
(let [{:keys [uri version default-option]} (:koodisto-source field)
empty-option {:value "" :label {:fi "" :sv "" :en ""}}
koodis (map (fn [koodi] (select-keys koodi [:value :label]))
empty-option {:value "" :label {:fi "" :sv "" :en ""}}
unsorted-koodis (map (fn [koodi] (select-keys koodi [:value :label]))
(get-koodisto-options koodisto-cache uri version (:allow-invalid? (:koodisto-source field))))
koodis-with-default-option (cond->> koodis
(some? default-option)
(map (fn [option]
(cond-> option
(= default-option (-> option :label :fi))
(assoc :default-value true)))))
koodis-with-followups (update-options-while-keeping-existing-followups koodis-with-default-option (:options field))]
(assoc field :options (if (= (:fieldType field) "dropdown")
(->> koodis-with-followups
koodis (sort-by (comp :fi :label) unsorted-koodis)
koodis-with-default-option (cond->> koodis
(some? default-option)
(map (fn [option]
(cond-> option
(= default-option (-> option :label :fi))
(assoc :default-value true)))))
removed-followup-values (removed-followup-values koodis-with-default-option
(:options field)
remove-existing)
koodis-with-followups (update-options-while-keeping-existing-followups koodis-with-default-option
(:options field)
remove-existing)]
(when (not-empty removed-followup-values)
(log/warn "Followup questions would be silently removed, field" (:id field) "values" removed-followup-values)
(throw (user-feedback-exception
(str "Kysymyksellä " (:id field) " on jatkokysymyksiä jotka poistettaisiin: " removed-followup-values))))
(assoc field :options (if (= (:fieldType field) "dropdown")
(->> koodis-with-followups
(remove (comp (partial = "") :value))
(cons empty-option))
koodis-with-followups))))
koodis-with-followups))))

(def attachment-type-koodisto-uri "liitetyypitamm")
(def attachment-type-koodisto-version 1)
Expand All @@ -72,21 +92,24 @@
(assoc field :label label)))

(defn- populate-form-koodisto-field
[koodisto-cache field]
[koodisto-cache remove-existing field]
(cond
(choice-field-from-koodisto? field)
(populate-choice-field-from-koodisto koodisto-cache field)
(populate-choice-field-from-koodisto koodisto-cache field remove-existing)

(attachment-from-kouta? field)
(populate-attachment-field-from-koodisto koodisto-cache field)

:else field))

(defn populate-form-koodisto-fields
[koodisto-cache form]
([koodisto-cache form remove-existing]
(update form :content (partial util/map-form-fields
(partial populate-form-koodisto-field
koodisto-cache))))
koodisto-cache
remove-existing))))
([koodisto-cache form]
(populate-form-koodisto-fields koodisto-cache form false)))

(defn get-postal-office-by-postal-code
[koodisto-cache postal-code]
Expand Down
5 changes: 5 additions & 0 deletions src/clj/ataru/virkailija/virkailija_routes.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@
:summary "Toggle form locked state"
(ok (access-controlled-form/update-form-lock id operation session tarjonta-service organization-service audit-logger)))

(api/PUT "/forms/:form-key/refresh-codes" {session :session}
:summary "Update all code values on form to latest ones from koodisto service"
:path-params [form-key :- s/Str]
(ok (access-controlled-form/refresh-form-codes form-key session tarjonta-service organization-service koodisto-cache audit-logger)))

(api/DELETE "/forms/:id" {session :session}
:path-params [id :- Long]
:summary "Mark form as deleted"
Expand Down
Loading
Loading