From c423ecd0e43fa186fe3f8300dc5b3fb3f3a627c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Severi=20J=C3=A4=C3=A4skel=C3=A4inen?= Date: Mon, 16 Dec 2024 17:35:38 +0200 Subject: [PATCH] Simplify error handling in `handle-palaute-waiting-for-vastaajatunnus!` --- src/oph/ehoks/db/dynamodb.clj | 68 +---- src/oph/ehoks/db/sql/palaute.sql | 5 + src/oph/ehoks/external/arvo.clj | 62 +++-- src/oph/ehoks/external/aws_sqs.clj | 2 +- src/oph/ehoks/external/koski.clj | 10 +- src/oph/ehoks/heratepalvelu.clj | 127 +++++++-- .../ehoks/heratepalvelu/herate_handler.clj | 3 +- src/oph/ehoks/palaute.clj | 41 ++- src/oph/ehoks/palaute/handler.clj | 8 +- src/oph/ehoks/palaute/scheduler.clj | 2 +- src/oph/ehoks/palaute/tapahtuma.clj | 2 + src/oph/ehoks/palaute/tyoelama.clj | 259 +++++++++--------- src/oph/ehoks/palaute/tyoelama/nippu.clj | 43 ++- test/oph/ehoks/external/koski_test.clj | 10 +- test/oph/ehoks/hoks/hoks_test_utils.clj | 3 + test/oph/ehoks/palaute/handler_test.clj | 3 +- .../oph/ehoks/palaute/tyoelama/nippu_test.clj | 36 +-- test/oph/ehoks/palaute/tyoelama_test.clj | 166 +++++++---- 18 files changed, 490 insertions(+), 360 deletions(-) diff --git a/src/oph/ehoks/db/dynamodb.clj b/src/oph/ehoks/db/dynamodb.clj index 6a7d6a888..d39fefc11 100644 --- a/src/oph/ehoks/db/dynamodb.clj +++ b/src/oph/ehoks/db/dynamodb.clj @@ -64,11 +64,19 @@ :jakso [:hankkimistapa_id] :nippu [:ohjaaja_ytunnus_kj_tutkinto :niputuspvm]}) +(defn jaksot [] (far/scan @faraday-opts @(tables :jakso) {})) +(defn niput [] (far/scan @faraday-opts @(tables :nippu) {})) + (defn get-item! "A wrapper for `taoensso.faraday/get-item`." [table prim-kvs] (far/get-item @faraday-opts @(tables table) prim-kvs)) +(defn delete-item! + "A wrapper for `taoensso.faraday/delete-item`." + [table prim-kvs] + (far/delete-item @faraday-opts @(tables table) prim-kvs)) + (defn sync-item! "Does a partial upsert on item in DDB: if the item doesn't exist, it is created with the given values. If it does exist, then only the @@ -129,63 +137,3 @@ :synkronointi-epaonnistui {:errormsg (.getMessage e) :body (:body (ex-data e))}) (throw e)))))) - -(def map-jakso-keys-to-ddb - (some-fn {:hankkimistapa-id :hankkimistapa_id, - :osaamisen-hankkimistapa-koodi-uri :hankkimistapa_tyyppi, - :hoks-id :hoks_id, - :jakso-alkupvm :jakso_alkupvm, - :jakso-loppupvm :jakso_loppupvm, - :ohjaaja-email :ohjaaja_email, - :ohjaaja-nimi :ohjaaja_nimi, - :ohjaaja-puhelinnumero :ohjaaja_puhelinnumero, - :ohjaaja-ytunnus-kj-tutkinto :ohjaaja_ytunnus_kj_tutkinto, - :opiskeluoikeus-oid :opiskeluoikeus_oid, - :oppija-oid :oppija_oid, - :oppisopimuksen-perusta-koodi-uri :oppisopimuksen_perusta, - :osa-aikaisuustieto :osa_aikaisuus, - :request-id :request_id, - :toimipiste-oid :toimipiste_oid, - :tutkinnonosa-id :tutkinnonosa_id, - :tutkinnon-osa-koodi-uri :tutkinnonosa_koodi, - :tutkinnonosa-nimi :tutkinnonosa_nimi, - :tutkinnonosa-tyyppi :tutkinnonosa_tyyppi, - :tyopaikan-nimi :tyopaikan_nimi, - :tyopaikan-normalisoitu-nimi :tyopaikan_normalisoitu_nimi, - :tyopaikan-y-tunnus :tyopaikan_ytunnus, - :vastuullinen-tyopaikka-ohjaaja-nimi :ohjaaja_nimi, - :vastuullinen-tyopaikka-ohjaaja-sahkoposti :ohjaaja_email, - :vastuullinen-tyopaikka-ohjaaja-puhelinnumero - :ohjaaja_puhelinnumero, - :viimeinen-vastauspvm :viimeinen_vastauspvm} - identity)) - -(defn sync-jakso-herate! - "Update the herätepalvelu jaksotunnustable to have the same content - for given heräte as palaute-backend has in its own database. - sync-jakso-herate! only updates fields it 'owns': currently that - means that the messaging tracking fields are left intact (because - herätepalvelu will update those)." - [tep-palaute] - (if-not (contains? (set (:heratepalvelu-responsibities config)) - :sync-jakso-heratteet) - (log/warn "sync-jakso-herate!: configured to not do anything") - (-> tep-palaute - utils/to-underscore-keys - ;; the only field that has dashes in its name is tpk-niputuspvm - (rename-keys {:tpk_niputuspvm :tpk-niputuspvm}) - (->> (sync-item! :jakso))))) - -(defn sync-tpo-nippu-herate! - "Update the Herätepalvelu nipputable to have the same content for given heräte - as palaute-backend has in its own database." - [nippu] - (if-not (contains? (set (:heratepalvelu-responsibities config)) - :sync-jakso-heratteet) - (log/warn "sync-tpo-nippu-herate!: configured to not do anything") - (if-let [existing-nippu (get-item! :nippu - (select-keys - nippu [:ohjaaja_ytunnus_kj_tutkinto - :niputuspvm]))] - (log/info "Tietokannassa on jo nippu: " existing-nippu) - (sync-item! :nippu nippu)))) diff --git a/src/oph/ehoks/db/sql/palaute.sql b/src/oph/ehoks/db/sql/palaute.sql index e01b27fb7..66913d339 100644 --- a/src/oph/ehoks/db/sql/palaute.sql +++ b/src/oph/ehoks/db/sql/palaute.sql @@ -68,6 +68,11 @@ where h.oppija_oid = :oppija-oid -- FIXME: should probably have deleted_at cond and p.kyselytyyppi in (:v*:kyselytyypit) and (p.koulutustoimija = :koulutustoimija or (:koulutustoimija)::text is null) +-- :name get-by-id! :? :1 +-- :doc Get palaute by palaute id. +select * from palautteet +where id = :id + -- :name get-by-hoks-id-and-yksiloiva-tunniste! :? :1 -- :doc Get palaute information for työpaikkajakso by HOKS ID and yksiloiva -- tunniste. diff --git a/src/oph/ehoks/external/arvo.clj b/src/oph/ehoks/external/arvo.clj index 45569cfe6..fe54c6022 100644 --- a/src/oph/ehoks/external/arvo.clj +++ b/src/oph/ehoks/external/arvo.clj @@ -54,55 +54,73 @@ (defn build-jaksotunnus-request-body "Luo dataobjektin TEP-jaksotunnuksen luomisrequestille." - [{:keys [opiskeluoikeus suoritus koulutustoimija toimipiste niputuspvm] + [{:keys [opiskeluoikeus existing-palaute suoritus koulutustoimija toimipiste + niputuspvm] :as ctx} - herate request-id] + request-id] {:koulutustoimija_oid koulutustoimija - :tyonantaja (:tyopaikan-y-tunnus herate) - :tyopaikka (:tyopaikan-nimi herate) - :tyopaikka_normalisoitu (utils/normalize-string (:tyopaikan-nimi herate)) + :tyonantaja (:tyopaikan-y-tunnus existing-palaute) + :tyopaikka (:tyopaikan-nimi existing-palaute) + :tyopaikka_normalisoitu (utils/normalize-string + (:tyopaikan-nimi existing-palaute)) :tutkintotunnus (get-in suoritus [:koulutusmoduuli :tunniste :koodiarvo]) - :tutkinnon_osa (when (:tutkinnon-osa-koodi-uri herate) + :tutkinnon_osa (when (:tutkinnon-osa-koodi-uri existing-palaute) (last (string/split - (:tutkinnon-osa-koodi-uri herate) + (:tutkinnon-osa-koodi-uri existing-palaute) #"_"))) - :paikallinen_tutkinnon_osa (:tutkinnonosa-nimi herate) + :paikallinen_tutkinnon_osa (:tutkinnonosa-nimi existing-palaute) :tutkintonimike (map :koodiarvo (:tutkintonimike suoritus)) :osaamisala (suoritus/get-osaamisalat suoritus (:oid opiskeluoikeus) - (:heratepvm herate)) - :tyopaikkajakson_alkupvm (str (:alkupvm herate)) - :tyopaikkajakson_loppupvm (str (:loppupvm herate)) - :rahoituskausi_pvm (str (:loppupvm herate)) - :osa_aikaisuus (:osa-aikaisuustieto herate) + (:heratepvm existing-palaute)) + :tyopaikkajakson_alkupvm (str (:alkupvm existing-palaute)) + :tyopaikkajakson_loppupvm (str (:loppupvm existing-palaute)) + :rahoituskausi_pvm (str (:loppupvm existing-palaute)) + :osa_aikaisuus (:osa-aikaisuustieto existing-palaute) :sopimustyyppi (last (string/split - (:osaamisen-hankkimistapa-koodi-uri herate) + (:osaamisen-hankkimistapa-koodi-uri + existing-palaute) #"_")) - :oppisopimuksen_perusta (when (:oppisopimuksen-perusta-koodi-uri herate) + :oppisopimuksen_perusta (when (:oppisopimuksen-perusta-koodi-uri + existing-palaute) (last (string/split - (:oppisopimuksen-perusta-koodi-uri herate) + (:oppisopimuksen-perusta-koodi-uri + existing-palaute) #"_"))) :vastaamisajan_alkupvm (str niputuspvm) :oppilaitos_oid (:oid (:oppilaitos opiskeluoikeus)) :toimipiste_oid toimipiste :request_id request-id}) -(defn create-jaksotunnus - [data] - (if-not (contains? (set (:arvo-responsibilities config)) :create-jaksotunnus) - (log/warn "create-jaksotunnus!: configured to not do anything") - (call! :post "/tyoelamapalaute/v1/vastaajatunnus" - {:form-params data :content-type :json}))) +(defn create-jaksotunnus! + [request] + (let [configured-to-call-arvo? (contains? + (set (:arvo-responsibilities config)) + :create-jaksotunnus) + response (when configured-to-call-arvo? + (call! :post "/tyoelamapalaute/v1/vastaajatunnus" + {:form-params request :content-type :json}))] + (if (:tunnus response) + response + (throw (ex-info + (if configured-to-call-arvo? + (format (str "No jaksotunnus got from Arvo with request %s. " + "Response from Arvo was %s.") + request response) + "create-jaksotunnus!: configured to not do anything.") + {:type ::no-jaksotunnus-created + :configured-to-call-arvo? configured-to-call-arvo?}))))) (defn delete-jaksotunnus [tunnus] + {:pre [(some? tunnus)]} (call! :delete (str "/tyoelamapalaute/v1/vastaajatunnus/" tunnus) {})) diff --git a/src/oph/ehoks/external/aws_sqs.clj b/src/oph/ehoks/external/aws_sqs.clj index 40577138c..54df0ff4a 100644 --- a/src/oph/ehoks/external/aws_sqs.clj +++ b/src/oph/ehoks/external/aws_sqs.clj @@ -153,7 +153,7 @@ (send-message {:kyselylinkki kyselylinkki} delete-tunnus-queue-url) (log/error "No AMIS delete tunnus queue!"))) -(defn send-tyoelamapalaute-message +(defn send-tyoelamapalaute-message! "Lähettää työelämäpalauteviestin." [msg] (if (contains? (set (:heratepalvelu-responsibities config)) diff --git a/src/oph/ehoks/external/koski.clj b/src/oph/ehoks/external/koski.clj index 7b23c24b2..8e22401bb 100644 --- a/src/oph/ehoks/external/koski.clj +++ b/src/oph/ehoks/external/koski.clj @@ -93,17 +93,21 @@ (try (get-opiskeluoikeus-info-raw oid) (catch ExceptionInfo e - (let [koski-virhekoodi (virhekoodi e)] - (when-not (and (= (:status (ex-data e)) status/not-found) + (let [http-status (:status (ex-data e)) + koski-virhekoodi (virhekoodi e)] + (when-not (and (= http-status status/not-found) (= koski-virhekoodi "notFound.opiskeluoikeuttaEiLöydyTaiEiOikeuksia")) (throw (ex-info (format (str "Error while fetching opiskeluoikeus `%s` " - "from Koski. Koski-virhekoodi is `%s`.") + "from Koski. Got response with HTTP status %d " + "and Koski-virhekoodi `%s`.") oid + http-status koski-virhekoodi) {:type ::opiskeluoikeus-fetching-error :opiskeluoikeus-oid oid + :http-status http-status :koski-virhekoodi koski-virhekoodi} e))))))) diff --git a/src/oph/ehoks/heratepalvelu.clj b/src/oph/ehoks/heratepalvelu.clj index 59c4dea5a..5eadf2b65 100644 --- a/src/oph/ehoks/heratepalvelu.clj +++ b/src/oph/ehoks/heratepalvelu.clj @@ -1,30 +1,27 @@ (ns oph.ehoks.heratepalvelu - (:require [clojure.string :as string] + (:require [clojure.set :refer [rename-keys]] + [clojure.string :as string] [clojure.tools.logging :as log] + [medley.core :refer [find-first]] + [oph.ehoks.config :refer [config]] [oph.ehoks.db.db-operations.hoks :as db-hoks] + [oph.ehoks.db.dynamodb :as ddb] [oph.ehoks.external.arvo :as arvo] [oph.ehoks.external.aws-sqs :as sqs] - [oph.ehoks.palaute.opiskelija.kyselylinkki :as kyselylinkki] + [oph.ehoks.opiskeluoikeus.suoritus :as suoritus] + [oph.ehoks.palaute :as palaute] [oph.ehoks.palaute.opiskelija :as op] - [oph.ehoks.palaute.tyoelama :as tep]) + [oph.ehoks.palaute.opiskelija.kyselylinkki :as kyselylinkki] + [oph.ehoks.palaute.tyoelama.nippu :as nippu] + [oph.ehoks.utils :as utils] + [oph.ehoks.utils.date :as date]) (:import (java.time LocalDate))) -(defn send-workplace-periods +(defn send-workplace-periods! "Formats and sends a list of periods to a SQS queue" [periods] (doseq [period periods] - (sqs/send-tyoelamapalaute-message (sqs/build-tyoelamapalaute-msg period)))) - -(defn process-finished-workplace-periods - "Finds all finished workplace periods between dates start and - end and sends them to a SQS queue" - [start end limit] - (let [periods (tep/finished-workplace-periods! start end limit)] - (log/infof - "Sending %d (limit %d) finished workplace periods between %s - %s" - (count periods) limit start end) - (send-workplace-periods periods) - periods)) + (sqs/send-tyoelamapalaute-message! (sqs/build-tyoelamapalaute-msg period)))) (defn get-oppija-kyselylinkit "Returns all feedback links for oppija" @@ -87,3 +84,101 @@ (db-hoks/select-tyoelamajaksot-active-between "hato" oppija start end) (db-hoks/select-tyoelamajaksot-active-between "hpto" oppija start end) (db-hoks/select-tyoelamajaksot-active-between "hyto" oppija start end))) + +(defn- add-keys + [palaute {:keys [opiskeluoikeus niputuspvm vastaamisajan-alkupvm] :as ctx} + request-id tunnus] + (let [niputuspvm niputuspvm + koulutustoimija (palaute/koulutustoimija-oid! opiskeluoikeus) + oo-suoritus (find-first suoritus/ammatillinen? + (:suoritukset opiskeluoikeus)) + tutkinto (get-in oo-suoritus + [:koulutusmoduuli :tunniste :koodiarvo])] + (assoc palaute + :tallennuspvm (date/now) + :alkupvm vastaamisajan-alkupvm + :koulutustoimija koulutustoimija + :niputuspvm niputuspvm + :ohjaaja-ytunnus-kj-tutkinto (nippu/tunniste ctx palaute) + :oppilaitos (:oid (:oppilaitos opiskeluoikeus)) + :osaamisala (str (seq (suoritus/get-osaamisalat + oo-suoritus (:oid opiskeluoikeus) + (:heratepvm palaute)))) + :request-id request-id + :toimipiste-oid (str (palaute/toimipiste-oid! oo-suoritus)) + :tpk-niputuspvm "ei_maaritelty" + :tunnus tunnus + :tutkinto tutkinto + :tutkintonimike (str (seq (map :koodiarvo + (:tutkintonimike oo-suoritus)))) + :tyopaikan-normalisoitu-nimi (utils/normalize-string + (:tyopaikan-nimi palaute)) + :viimeinen-vastauspvm + (str (.plusDays ^LocalDate vastaamisajan-alkupvm 60))))) + +; Helper functions that can be mocked in tests +(def sync-jakso!* (partial ddb/sync-item! :jakso)) +(def sync-tpo-nippu!* (partial ddb/sync-item! :nippu)) + +;; FIXME: tältä puuttuu yksikkötesti. +;; test-create-and-save-arvo-vastaajatunnus-for-all-needed! sisältää +;; ylimalkaisen testin tälle funktiolle. +(defn sync-jakso! + "Update the herätepalvelu jaksotunnustable to have the same content + for given heräte as palaute-backend has in its own database. + sync-jakso-herate! only updates fields it 'owns': currently that + means that the messaging tracking fields are left intact (because + herätepalvelu will update those)." + [{:keys [existing-palaute tx] :as ctx} request-id tunnus] + {:pre [(some? tunnus)]} + (if-not (contains? (set (:heratepalvelu-responsibities config)) + :sync-jakso-heratteet) + (log/warn "sync-jakso!: configured to not do anything") + (let [query (select-keys existing-palaute + [:jakson-yksiloiva-tunniste :hoks-id])] + (try (-> (palaute/get-for-heratepalvelu-by-hoks-id-and-yksiloiva-tunniste! + tx query) + (first) + (not-empty) + (or (throw (ex-info "palaute not found" query))) + (add-keys ctx request-id tunnus) + (dissoc :internal-kyselytyyppi :jakson-yksiloiva-tunniste) + (utils/remove-nils) + utils/to-underscore-keys + ;; the only field that has dashes in its name is tpk-niputuspvm + (rename-keys {:tpk_niputuspvm :tpk-niputuspvm}) + (sync-jakso!*)) + (catch Exception e + (throw (ex-info (format (str "Failed to sync jakso `%s` of HOKS " + "`%d` to Herätepalvelu") + (:jakson-yksiloiva-tunniste query) + (:hoks-id query)) + {:type ::jakso-sync-failed + :arvo-tunnus tunnus} + e))))))) + +(defn sync-tpo-nippu! + "Update the Herätepalvelu nipputable to have the same content for given heräte + as palaute-backend has in its own database." + [nippu tunnus] + {:pre [(:koulutuksenjarjestaja nippu) (:niputuspvm nippu) (:tutkinto nippu)]} + (if-not (contains? (set (:heratepalvelu-responsibities config)) + :sync-jakso-heratteet) + (log/warn "sync-tpo-nippu!: configured to not do anything") + (let [tunnisteet (select-keys nippu [:ohjaaja_ytunnus_kj_tutkinto + :niputuspvm])] + (try + (if (ddb/get-item! :nippu tunnisteet) + (log/infof "Nippu `%s` already exists" tunnisteet) + (sync-tpo-nippu!* nippu)) + (catch Exception e + (throw (ex-info (format (str "Failed to sync TPO-nippu with " + "tunnisteet %s to Herätepalvelu") + tunnisteet) + {:type ::tpo-nippu-sync-failed + :arvo-tunnus tunnus} + e))))))) + +(defn delete-jakso-herate! + [tep-palaute] + (ddb/delete-item! :jakso {:hankkimistapa_id (:hankkimistapa-id tep-palaute)})) diff --git a/src/oph/ehoks/heratepalvelu/herate_handler.clj b/src/oph/ehoks/heratepalvelu/herate_handler.clj index 966a3b95c..a59f1f68f 100644 --- a/src/oph/ehoks/heratepalvelu/herate_handler.clj +++ b/src/oph/ehoks/heratepalvelu/herate_handler.clj @@ -3,6 +3,7 @@ [compojure.api.sweet :as c-api] [oph.ehoks.db.db-operations.hoks :as db-hoks] [oph.ehoks.heratepalvelu :as hp] + [oph.ehoks.palaute.tyoelama :as tep] [oph.ehoks.hoks :as hoks] [oph.ehoks.hoks.middleware :as m] [oph.ehoks.logging.audit :as audit] @@ -33,7 +34,7 @@ limit :- (s/maybe s/Int)] :return (restful/response s/Int) (let [l (or limit 10) - periods (hp/process-finished-workplace-periods start end l)] + periods (tep/process-finished-workplace-periods! start end l)] (restful/ok (count periods)))) (c-api/GET "/kasittelemattomat-heratteet" [] diff --git a/src/oph/ehoks/palaute.clj b/src/oph/ehoks/palaute.clj index 060461d05..7b4d4ffa0 100644 --- a/src/oph/ehoks/palaute.clj +++ b/src/oph/ehoks/palaute.clj @@ -1,8 +1,10 @@ (ns oph.ehoks.palaute - (:require [clojure.set :refer [rename-keys]] + (:require [clojure.java.jdbc :as jdbc] + [clojure.set :refer [rename-keys]] [clojure.tools.logging :as log] [hugsql.core :as hugsql] [medley.core :refer [assoc-some find-first]] + [oph.ehoks.db :as db] [oph.ehoks.external.koski :as koski] [oph.ehoks.external.organisaatio :as organisaatio] [oph.ehoks.hoks :as hoks] @@ -135,6 +137,13 @@ (let [db-handler (if (:id palaute) update! insert!)] (db-handler tx palaute))) +(defn update-tila! + [{:keys [existing-palaute] :as ctx} tila reason lisatiedot] + (jdbc/with-db-transaction + [tx db/spec] + (update! tx {:id (:id existing-palaute) :tila tila}) + (tapahtuma/build-and-insert! ctx "ei_laheteta" reason lisatiedot))) + (defn feedback-collecting-prevented? "Jätetäänkö palaute keräämättä sen vuoksi, että opiskelijan opiskelu on tällä hetkellä rahoitettu muilla rahoituslähteillä?" @@ -207,19 +216,23 @@ [:ei-laheteta :opiskeluoikeus-oid :liittyva-opiskeluoikeus]))) (defn save-arvo-tunniste! - [{:keys [tx existing-palaute] :as ctx} arvo-vastaus] + [{:keys [tx existing-palaute] :as ctx} arvo-response] + {:pre [(:tunnus arvo-response)]} (let [new-state (if (= (:kyselytyyppi existing-palaute) "tyopaikkajakson_suorittaneet") :vastaajatunnus-muodostettu :kysely-muodostettu)] - (-> arvo-vastaus - (rename-keys {:kysely_linkki :url}) - (assoc :id (:id existing-palaute) - :tila (utils/to-underscore-str new-state)) - (update :url identity) ; ensure key exists - (->> (update-arvo-tunniste! tx)) - (assert)) - (tapahtuma/build-and-insert! - ctx - new-state - :arvo-kutsu-onnistui - {:arvo_response arvo-vastaus}))) + (try (-> arvo-response + (rename-keys {:kysely_linkki :url}) + (assoc :id (:id existing-palaute) + :tila (utils/to-underscore-str new-state)) + (update :url identity) ; ensure key exists + (->> (update-arvo-tunniste! tx)) + (assert)) + (tapahtuma/build-and-insert! + ctx new-state :arvo-kutsu-onnistui {:arvo_response arvo-response}) + (catch Exception e + (throw (ex-info "Failed to save Arvo-tunnus to DB" + {:type :failed-to-save-arvo-tunnus + :arvo-tunnus (:tunnus arvo-response)} + e))))) + (:tunnus arvo-response)) diff --git a/src/oph/ehoks/palaute/handler.clj b/src/oph/ehoks/palaute/handler.clj index aedfcd6b6..da3555dd4 100644 --- a/src/oph/ehoks/palaute/handler.clj +++ b/src/oph/ehoks/palaute/handler.clj @@ -65,8 +65,7 @@ :header-params [caller-id :- s/Str ticket :- s/Str] (let [vastaajatunnukset - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! - {})] + (tep/handle-all-palautteet-waiting-for-vastaajatunnus! {})] (assoc (restful/ok {:vastaajatunnukset vastaajatunnukset}) ::audit/target {:vastaajatunnukset vastaajatunnukset}))) @@ -79,8 +78,9 @@ (if-let [tep-palaute (palaute/get-tep-palaute-waiting-for-vastaajatunnus! db/spec {:palaute-id palaute-id})] - (let [vastaajatunnus (tep/create-and-save-arvo-vastaajatunnus! - tep-palaute)] + (let [vastaajatunnus + (tep/handle-palaute-waiting-for-vastaajatunnus! + tep-palaute)] (assoc (restful/ok {:vastaajatunnus vastaajatunnus}) ::audit/target {:vastaajatunnus vastaajatunnus :palaute-id palaute-id})) diff --git a/src/oph/ehoks/palaute/scheduler.clj b/src/oph/ehoks/palaute/scheduler.clj index 42b893d45..0124deaf0 100644 --- a/src/oph/ehoks/palaute/scheduler.clj +++ b/src/oph/ehoks/palaute/scheduler.clj @@ -9,7 +9,7 @@ "Run these tasks sequentially." [opts] (amis/create-and-save-arvo-kyselylinkki-for-all-needed! opts) - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! opts)) + (tep/handle-all-palautteet-waiting-for-vastaajatunnus! opts)) (defn run-scheduler! "Simple (daily) scheduler for palaute scheduled tasks. Will be replaced with diff --git a/src/oph/ehoks/palaute/tapahtuma.clj b/src/oph/ehoks/palaute/tapahtuma.clj index 1372142d1..4f401b9af 100644 --- a/src/oph/ehoks/palaute/tapahtuma.clj +++ b/src/oph/ehoks/palaute/tapahtuma.clj @@ -8,8 +8,10 @@ (defn build ([{:keys [existing-palaute] :as ctx} reason lisatiedot] (build ctx (:tila existing-palaute) reason lisatiedot nil)) + ([ctx state reason lisatiedot] (build ctx state reason lisatiedot nil)) + ([{:keys [tapahtumatyyppi existing-palaute] :as ctx} state reason lisatiedot palaute] {:pre [(some? tapahtumatyyppi) (some? state)]} diff --git a/src/oph/ehoks/palaute/tyoelama.clj b/src/oph/ehoks/palaute/tyoelama.clj index 84c11db05..7bb18d991 100644 --- a/src/oph/ehoks/palaute/tyoelama.clj +++ b/src/oph/ehoks/palaute/tyoelama.clj @@ -1,19 +1,18 @@ (ns oph.ehoks.palaute.tyoelama (:require [clojure.java.jdbc :as jdbc] [clojure.tools.logging :as log] + [clojure.walk :refer [walk]] [medley.core :refer [find-first map-vals]] - [oph.ehoks.config :refer [config]] [oph.ehoks.db :as db] [oph.ehoks.db.db-operations.hoks :as db-hoks] - [oph.ehoks.db.dynamodb :as ddb] [oph.ehoks.external.arvo :as arvo] [oph.ehoks.external.koski :as koski] + [oph.ehoks.heratepalvelu :as heratepalvelu] [oph.ehoks.hoks.osaamisen-hankkimistapa :as oht] [oph.ehoks.opiskeluoikeus.suoritus :as suoritus] [oph.ehoks.palaute :as palaute] [oph.ehoks.palaute.tapahtuma :as tapahtuma] [oph.ehoks.palaute.tyoelama.nippu :as nippu] - [oph.ehoks.utils :as utils] [oph.ehoks.utils.date :as date]) (:import (clojure.lang ExceptionInfo) (java.time LocalDate) @@ -27,6 +26,17 @@ hatos (db-hoks/select-paattyneet-tyoelamajaksot "hato" start end limit)] (concat hytos hptos hatos))) +(defn process-finished-workplace-periods! + "Finds all finished workplace periods between dates start and + end and sends them to a SQS queue" + [start end limit] + (let [periods (finished-workplace-periods! start end limit)] + (log/infof + "Sending %d (limit %d) finished workplace periods between %s - %s" + (count periods) limit start end) + (heratepalvelu/send-workplace-periods! periods) + periods)) + (defn tyopaikkajaksot "Takes `hoks` as an input and extracts from it all osaamisen hankkimistavat that are tyopaikkajaksos. Returns a lazy sequence." @@ -130,139 +140,128 @@ [{:keys [hoks] :as ctx}] (run! #(initiate-if-needed! ctx %) (tyopaikkajaksot hoks))) -(defn add-keys - [tep-palaute {:keys [opiskeluoikeus niputuspvm] :as ctx} request-id tunnus] - ;; TODO: check this niputuspvm rule when palaute-backend is - ;; responsible for niputus - (let [niputuspvm niputuspvm - alkupvm (next-niputus-date - (LocalDate/parse (:jakso-loppupvm tep-palaute))) - koulutustoimija (palaute/koulutustoimija-oid! opiskeluoikeus) - oo-suoritus (find-first suoritus/ammatillinen? - (:suoritukset opiskeluoikeus)) - tutkinto (get-in oo-suoritus - [:koulutusmoduuli :tunniste :koodiarvo])] - (assoc tep-palaute - :tallennuspvm (date/now) - :alkupvm alkupvm - :koulutustoimija koulutustoimija - :niputuspvm niputuspvm - :ohjaaja-ytunnus-kj-tutkinto (nippu/tunniste ctx tep-palaute) - :oppilaitos (:oid (:oppilaitos opiskeluoikeus)) - :osaamisala (str (seq (suoritus/get-osaamisalat - oo-suoritus (:oid opiskeluoikeus) - (:heratepvm tep-palaute)))) - :request-id request-id - :toimipiste-oid (str (palaute/toimipiste-oid! oo-suoritus)) - :tpk-niputuspvm "ei_maaritelty" - :tunnus tunnus - :tutkinto tutkinto - :tutkintonimike (str (seq (map :koodiarvo - (:tutkintonimike oo-suoritus)))) - :tyopaikan-normalisoitu-nimi (utils/normalize-string - (:tyopaikan-nimi tep-palaute)) - :viimeinen-vastauspvm (str (.plusDays alkupvm 60))))) +(defn- create-and-save-arvo-tunnus! + "Makes a request to Arvo for jaksotunnus creation and saves a successfully + created jaksotunnus to DB." + [ctx request-id] + (->> (arvo/build-jaksotunnus-request-body ctx request-id) + (arvo/create-jaksotunnus!) + (palaute/save-arvo-tunniste! ctx))) + +(defn- throw-if-keskeytynyt! + "Checks if jakso corresponding to `tep-palaute` is keskeytynyt (i.e., has + one or more open keskeytymisajanjakso), in which case the function throws an + exception, interrupting further palaute processing." + [tx tep-palaute] + (let [keskeytymisajanjaksot (oht/get-keskeytymisajanjaksot! + tx {:oht-id (:hankkimistapa-id tep-palaute)})] + (when (not-every? #(some? (:loppu %)) keskeytymisajanjaksot) + (throw (ex-info (format (str "Jakso `%s` of HOKS `%d` has one or more " + "open keskeytymisajanjakso") + (:jakson-yksiloiva-tunniste tep-palaute) + (:hoks-id tep-palaute)) + {:type ::jakso-keskeytynyt + :keskeytymisajanjaksot keskeytymisajanjaksot}))))) + +(defn- enrich-ctx! + [ctx tep-palaute] + (let [opiskeluoikeus (koski/get-existing-opiskeluoikeus! + (:opiskeluoikeus-oid tep-palaute)) + suoritus (find-first suoritus/ammatillinen? + (:suoritukset opiskeluoikeus))] + (assoc ctx + :opiskeluoikeus opiskeluoikeus + :suoritus suoritus + :koulutustoimija (palaute/koulutustoimija-oid! opiskeluoikeus) + :toimipiste (palaute/toimipiste-oid! suoritus) + :niputuspvm (next-niputus-date (date/now)) + :vastaamisajan-alkupvm (next-niputus-date (:loppupvm tep-palaute))))) + +(defn- handle-exception + "Handles an ExceptionInfo `ex` based on `:type` in `ex-data`. If `ex` doesn't + match any of the cases below, re-throws `ex`." + [{:keys [existing-palaute] :as ctx} ex] + (let [ex-data (ex-data ex) + ex-type (:type ex-data) + tunnus (:arvo-tunnus ex-data)] + (when tunnus + (log/infof "Trying to delete jaksotunnus `%s` from Arvo" tunnus) + (arvo/delete-jaksotunnus tunnus)) + (case ex-type + ::koski/opiskeluoikeus-not-found + (do (log/warnf "%s. Setting `tila` to \"ei_laheteta\" for palaute `%d`." + (ex-message ex) + (:id existing-palaute)) + (palaute/update-tila! + ctx "ei_laheteta" ex-type (select-keys existing-palaute + [:opiskeluoikeus-oid]))) + + ::jakso-keskeytynyt + (do (log/warnf "%s. Setting `tila` to `ei_laheteta` for palaute `%d`. " + (ex-message ex) + (:id existing-palaute)) + (palaute/update-tila! + ctx "ei_laheteta" ex-type + (walk (fn [[k v]] [(name k) (str v)]) + identity + (select-keys ex-data [:keskeytymisajanjaksot])))) + + ::arvo/no-jaksotunnus-created + (log/logf (if (:configured-to-call-arvo? ex-data) :error :warn) + (str (ex-message ex) " Skipping processing for palaute `%d`") + (:id existing-palaute)) -;; FIXME: tältä puuttuu yksikkötesti. -;; test-create-and-save-arvo-vastaajatunnus-for-all-needed! sisältää -;; ylimalkaisen testin tälle funktiolle. -(defn sync-jakso-to-heratepalvelu! - [tx ctx tep-palaute request-id tunnus] - (let [query (select-keys tep-palaute [:jakson-yksiloiva-tunniste :hoks-id])] - (-> (palaute/get-for-heratepalvelu-by-hoks-id-and-yksiloiva-tunniste! - tx query) - (first) - (not-empty) - (or (throw (ex-info "palaute not found" query))) - (add-keys ctx request-id tunnus) - (dissoc :internal-kyselytyyppi :jakson-yksiloiva-tunniste) - (utils/remove-nils) - (ddb/sync-jakso-herate!)))) + ::heratepalvelu/tpo-nippu-sync-failed + (do (log/errorf (str "%s. Trying to delete jakso corresponding to " + "palaute `%d` from Herätepalvelu") + (ex-message ex) + (:id existing-palaute)) + (heratepalvelu/delete-jakso-herate! existing-palaute) + (throw ex)) -(defn create-and-save-arvo-vastaajatunnus! + ;; FIXME: tapahtuma) + (throw ex)))) + +(defn handle-palaute-waiting-for-vastaajatunnus! + "Creates vastaajatunnus for `tep-palaute`, then synchronizes the palaute + palaute and the corresponding TPO-nippu to Herätepalvelu." [tep-palaute] {:pre [(:vastuullinen-tyopaikka-ohjaaja-nimi tep-palaute) (:tyopaikan-nimi tep-palaute) (:tyopaikan-y-tunnus tep-palaute)]} - (if-let [opiskeluoikeus (koski/get-opiskeluoikeus! - (:opiskeluoikeus-oid tep-palaute))] - (let [suoritus (find-first suoritus/ammatillinen? - (:suoritukset opiskeluoikeus)) - ctx {:tapahtumatyyppi :arvo-luonti - :existing-palaute tep-palaute - :opiskeluoikeus opiskeluoikeus - :suoritus suoritus - :koulutustoimija (palaute/koulutustoimija-oid! opiskeluoikeus) - :toimipiste (palaute/toimipiste-oid! suoritus) - :niputuspvm (next-niputus-date (date/now))}] - ; The following attributes are required for TPO-nippu. - (assert (and (:koulutustoimija ctx) (:niputuspvm ctx) (:suoritus ctx))) - (jdbc/with-db-transaction - [tx db/spec] - (let [ctx (assoc ctx :tx tx) - keskeytymisajanjaksot - (oht/get-keskeytymisajanjaksot! - tx {:oht-id (:hankkimistapa-id tep-palaute)}) - nippu (nippu/build-tpo-nippu-for-heratepalvelu - ctx tep-palaute keskeytymisajanjaksot)] - (if (= (:kasittelytila nippu) "ei_niputettu") - (let [request-id (str (UUID/randomUUID)) - arvo-request (arvo/build-jaksotunnus-request-body - ctx tep-palaute request-id) - arvo-response (arvo/create-jaksotunnus arvo-request) - tunnus (:tunnus arvo-response)] - (try - (if-not tunnus - (log/warn "No vastaajatunnus got from arvo," - "so not marking handled") - (do (palaute/save-arvo-tunniste! ctx arvo-response) - (sync-jakso-to-heratepalvelu! - tx ctx tep-palaute request-id tunnus) - (assert - (oht/update! - tx {:id (:hankkimistapa-id tep-palaute) - :tep-kasitelty true})))) - (ddb/sync-tpo-nippu-herate! nippu) + (let [ctx {:tapahtumatyyppi :arvo-luonti + :existing-palaute tep-palaute}] + (try + (let [ctx (enrich-ctx! ctx tep-palaute)] + (jdbc/with-db-transaction + [tx db/spec] + (throw-if-keskeytynyt! tx tep-palaute) + (let [ctx (assoc ctx :tx tx) + request-id (str (UUID/randomUUID)) + tunnus (create-and-save-arvo-tunnus! ctx request-id)] + (heratepalvelu/sync-jakso! ctx request-id tunnus) + ; FIXME poista tep_kasitelty-logiikka siirtymävaiheen jälkeen? + (assert (oht/update! tx {:id (:hankkimistapa-id tep-palaute) + :tep-kasitelty true})) + ; TODO lisää nipputunniste palautteet-tauluun (uusi kantamigraatio) + ; TODO lisää uusi nippu jos sitä ei löydy nipputunnisteella kannasta + (heratepalvelu/sync-tpo-nippu! + (nippu/build-tpo-nippu-for-heratepalvelu ctx) tunnus) + tunnus))) + (catch ExceptionInfo e + (handle-exception ctx e))))) - ; TODO lisää nipputunniste palautteet-tauluun (uusi - ; kantamigraatio) - ; TODO lisää uusi nippu, jos sitä ei löydy nipputunnisteella - ; kannasta - - ; Aseta tep_kasitelty arvoon true jotta herätepalvelu ei yritä - ; tehdä vastaavaa operaatiota siirtymävaiheen/asennuksien - ; aikana. - ;FIXME poista tep_kasitelty-logiikka siirtymävaiheen jälkeen? - tunnus - (catch ExceptionInfo e - (log/errorf - e - (str "Error updating palaute %s, trying to remove " - "vastaajatunnus from Arvo: %s") - (:id tep-palaute) - tunnus) - ;; FIXME: tapahtuma - (when tunnus (arvo/delete-jaksotunnus tunnus)) - (throw e)))) - (ddb/sync-tpo-nippu-herate! nippu))))) - (log/warnf - "Opiskeluoikeus not found for palaute %d, skipping processing" - (:id tep-palaute)))) ; FIXME: create tapahtuma and update state - -(defn create-and-save-arvo-vastaajatunnus-for-all-needed! +(defn handle-all-palautteet-waiting-for-vastaajatunnus! "Creates vastaajatunnus for all herates that are waiting for processing, do not have vastaajatunnus and have heratepvm today or in the past." [_] - (if-not (contains? (set (:arvo-responsibilities config)) :create-jaksotunnus) - (log/warn "`create-and-save-arvo-vastaajatunnus-for-all-needed!` " - "configured not to do anything") - (do (log/info "Creating vastaajatunnus for unprocessed työelämäpalaute.") - (->> (palaute/get-tep-palautteet-waiting-for-vastaajatunnus! - db/spec {:heratepvm (str (date/now))}) - (map (fn [tep-palaute] - (try (log/infof "Creating vastaajatunnus for %d" - (:id tep-palaute)) - (create-and-save-arvo-vastaajatunnus! tep-palaute) - (catch ExceptionInfo e - (log/errorf e "Error processing tep-palaute %s" - tep-palaute))))) - doall)))) + (log/info "Creating vastaajatunnus for unprocessed työelämäpalaute.") + (->> (palaute/get-tep-palautteet-waiting-for-vastaajatunnus! + db/spec {:heratepvm (str (date/now))}) + (map (fn [tep-palaute] + (try + (log/infof "Creating vastaajatunnus for %d" (:id tep-palaute)) + (handle-palaute-waiting-for-vastaajatunnus! tep-palaute) + (catch ExceptionInfo e + (log/errorf e "Error processing tep-palaute %s" + tep-palaute))))) + doall)) diff --git a/src/oph/ehoks/palaute/tyoelama/nippu.clj b/src/oph/ehoks/palaute/tyoelama/nippu.clj index 9d56324c1..de6fd3fe1 100644 --- a/src/oph/ehoks/palaute/tyoelama/nippu.clj +++ b/src/oph/ehoks/palaute/tyoelama/nippu.clj @@ -1,6 +1,5 @@ (ns oph.ehoks.palaute.tyoelama.nippu - (:require [clojure.tools.logging :as log] - [oph.ehoks.utils :as utils])) + (:require [oph.ehoks.utils :as utils])) (defn tunniste [ctx tep-palaute] @@ -19,27 +18,21 @@ (get-in (:suoritus ctx) [:koulutusmoduuli :tunniste :koodiarvo]))) (defn build-tpo-nippu-for-heratepalvelu - [{:keys [suoritus koulutustoimija niputuspvm] :as ctx} - tep-palaute keskeytymisajanjaksot] - {:pre [(:tyopaikan-nimi tep-palaute)]} + [{:keys [existing-palaute suoritus koulutustoimija niputuspvm] :as ctx}] + {:pre [(:tyopaikan-nimi existing-palaute)]} (let [tutkinto (get-in suoritus [:koulutusmoduuli :tunniste :koodiarvo]) - tyopaikan-nimi (:tyopaikan-nimi tep-palaute) - tyopaikan-y-tunnus (:tyopaikan-y-tunnus tep-palaute) - ohjaaja (:vastuullinen-tyopaikka-ohjaaja-nimi tep-palaute) - nippu-data {:ohjaaja-ytunnus-kj-tutkinto (tunniste ctx tep-palaute) - :ohjaaja ohjaaja - :tyopaikka tyopaikan-nimi - :ytunnus tyopaikan-y-tunnus - :koulutuksenjarjestaja koulutustoimija - :tutkinto tutkinto - :kasittelytila "ei_niputettu" - :sms-kasittelytila "ei_lahetetty" - :niputuspvm niputuspvm}] - (utils/to-underscore-keys - (utils/remove-nils - (if (not-every? #(some? (:loppu %)) keskeytymisajanjaksot) - (do (log/info "Jakso on keskeytynyt, tätä ei niputeta.") - (assoc nippu-data - :kasittelytila "ei_niputeta" - :sms-kasittelytila "ei_niputeta")) - nippu-data))))) + tyopaikan-nimi (:tyopaikan-nimi existing-palaute) + tyopaikan-y-tunnus (:tyopaikan-y-tunnus existing-palaute) + ohjaaja (:vastuullinen-tyopaikka-ohjaaja-nimi + existing-palaute)] + (-> {:ohjaaja-ytunnus-kj-tutkinto (tunniste ctx existing-palaute) + :ohjaaja ohjaaja + :tyopaikka tyopaikan-nimi + :ytunnus tyopaikan-y-tunnus + :koulutuksenjarjestaja koulutustoimija + :tutkinto tutkinto + :kasittelytila "ei_niputettu" + :sms-kasittelytila "ei_lahetetty" + :niputuspvm niputuspvm} + (utils/remove-nils) + (utils/to-underscore-keys)))) diff --git a/test/oph/ehoks/external/koski_test.clj b/test/oph/ehoks/external/koski_test.clj index 72e2f7dcb..fc6376be1 100644 --- a/test/oph/ehoks/external/koski_test.clj +++ b/test/oph/ehoks/external/koski_test.clj @@ -63,8 +63,9 @@ ExceptionInfo (re-pattern (str "Error while fetching opiskeluoikeus " - "`1.246.562.15.12345678910` from Koski. " - "Koski-virhekoodi is `badRequest.format.number`.")) + "`1.246.562.15.12345678910` from Koski. Got response with " + "HTTP status 400 and Koski-virhekoodi " + "`badRequest.format.number`.")) (k/get-opiskeluoikeus! "1.246.562.15.12345678910")))))) (deftest test-get-existing-opiskeluoikeus! @@ -80,8 +81,9 @@ ExceptionInfo (re-pattern (str "Error while fetching opiskeluoikeus " - "`1.246.562.15.12345678910` from Koski. " - "Koski-virhekoodi is `badRequest.format.number`.")) + "`1.246.562.15.12345678910` from Koski. Got response with " + "HTTP status 400 and Koski-virhekoodi " + "`badRequest.format.number`.")) (k/get-opiskeluoikeus! "1.246.562.15.12345678910")))))) (deftest test-get-oppija-opiskeluoikeudet diff --git a/test/oph/ehoks/hoks/hoks_test_utils.clj b/test/oph/ehoks/hoks/hoks_test_utils.clj index f883d2ae8..f318d5295 100644 --- a/test/oph/ehoks/hoks/hoks_test_utils.clj +++ b/test/oph/ehoks/hoks/hoks_test_utils.clj @@ -151,6 +151,9 @@ base-url (dissoc hoks-test/hoks-1 :id)))) +(defn palautteet [] + (db-helpers/query ["select * from palautteet"])) + (defn kasittelemattomat-palauteet [] (db-helpers/query [(str "select * from palautteet " diff --git a/test/oph/ehoks/palaute/handler_test.clj b/test/oph/ehoks/palaute/handler_test.clj index f649c08e5..42a365c97 100644 --- a/test/oph/ehoks/palaute/handler_test.clj +++ b/test/oph/ehoks/palaute/handler_test.clj @@ -91,7 +91,8 @@ hoks-utils/mock-get-organisaatio! koski/get-opiskeluoikeus! hoks-utils/mock-get-opiskeluoikeus! - arvo/create-jaksotunnus hoks-utils/mock-create-jaksotunnus + arvo/create-jaksotunnus! + hoks-utils/mock-create-jaksotunnus date/now #(LocalDate/of 2024 6 30)] (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5)) diff --git a/test/oph/ehoks/palaute/tyoelama/nippu_test.clj b/test/oph/ehoks/palaute/tyoelama/nippu_test.clj index 4d41d657f..ca9596f0e 100644 --- a/test/oph/ehoks/palaute/tyoelama/nippu_test.clj +++ b/test/oph/ehoks/palaute/tyoelama/nippu_test.clj @@ -16,32 +16,14 @@ :niputuspvm (LocalDate/of 2024 12 16)}) (deftest test-build-tpo-nippu-for-heratepalvelu - (let [ctx {:koulutustoimija "1.2.246.562.10.346830761110" - :suoritus {:koulutusmoduuli - {:tunniste {:koodiarvo "12345"}}} - :niputuspvm (LocalDate/of 2024 12 16)} - tep-palaute + (let [tep-palaute {:vastuullinen-tyopaikka-ohjaaja-nimi "Matti Meikäläinen" :tyopaikan-y-tunnus "1234567-1" - :tyopaikan-nimi "Meikäläisen Murkinat Oy"}] - (testing "Kasittelytila for nippu will be" - (testing "\"ei_niputettu\" when" - (testing "there are no keskeytymisajanjaksos" - (is (= (nippu/build-tpo-nippu-for-heratepalvelu ctx tep-palaute {}) - expected-tpo-nippu-data))) - (testing "there are keskeytymisajanjaksos but they're all closed" - (is (= (nippu/build-tpo-nippu-for-heratepalvelu - ctx tep-palaute [{:alku (LocalDate/of 2023 11 1) - :loppu (LocalDate/of 2023 11 16)} - {:alku (LocalDate/of 2024 02 5) - :loppu (LocalDate/of 2024 02 8)}]) - expected-tpo-nippu-data)))) - (testing - "\"ei_niputeta\" when there are one or more open keskeytymisajanjakso" - (is (= (nippu/build-tpo-nippu-for-heratepalvelu - ctx tep-palaute [{:alku (LocalDate/of 2023 11 1) - :loppu (LocalDate/of 2023 11 16)} - {:alku (LocalDate/of 2024 02 5)}]) - (assoc expected-tpo-nippu-data - :kasittelytila "ei_niputeta" - :sms_kasittelytila "ei_niputeta"))))))) + :tyopaikan-nimi "Meikäläisen Murkinat Oy"} + ctx {:existing-palaute tep-palaute + :koulutustoimija "1.2.246.562.10.346830761110" + :suoritus {:koulutusmoduuli + {:tunniste {:koodiarvo "12345"}}} + :niputuspvm (LocalDate/of 2024 12 16)}] + (is (= (nippu/build-tpo-nippu-for-heratepalvelu ctx) + expected-tpo-nippu-data)))) diff --git a/test/oph/ehoks/palaute/tyoelama_test.clj b/test/oph/ehoks/palaute/tyoelama_test.clj index 357c646a4..b816455aa 100644 --- a/test/oph/ehoks/palaute/tyoelama_test.clj +++ b/test/oph/ehoks/palaute/tyoelama_test.clj @@ -1,9 +1,8 @@ (ns oph.ehoks.palaute.tyoelama-test - (:require [clojure.test :refer [are deftest is testing use-fixtures]] + (:require [clojure.set :as s] + [clojure.test :refer [are deftest is testing use-fixtures]] [clojure.tools.logging.test :refer [logged? with-log]] - [clojure.set :as s] [medley.core :refer [remove-vals]] - [taoensso.faraday :as far] [oph.ehoks.db :as db] [oph.ehoks.db.db-operations.db-helpers :as db-helpers] [oph.ehoks.db.db-operations.hoks :as db-hoks] @@ -12,15 +11,20 @@ [oph.ehoks.external.koski :as koski] [oph.ehoks.external.organisaatio :as organisaatio] [oph.ehoks.external.organisaatio-test :as organisaatio-test] + [oph.ehoks.heratepalvelu :as heratepalvelu] [oph.ehoks.hoks-test :as hoks-test] [oph.ehoks.hoks.hoks-test-utils :as hoks-utils] + [oph.ehoks.hoks.osaamisen-hankkimistapa :as oht] [oph.ehoks.opiskeluoikeus-test :as oo-test] [oph.ehoks.palaute :as palaute] [oph.ehoks.palaute.tapahtuma :as tapahtuma] [oph.ehoks.palaute.tyoelama :as tep] [oph.ehoks.test-utils :as test-utils] - [oph.ehoks.utils.date :as date]) - (:import (java.time LocalDate))) + [oph.ehoks.utils.date :as date] + [taoensso.faraday :as far]) + (:import [clojure.lang ExceptionInfo] + (java.time LocalDate) + (java.util UUID))) (use-fixtures :once test-utils/migrate-database) (use-fixtures :each test-utils/empty-database-after-test) @@ -300,6 +304,13 @@ (far/delete-item @ddb/faraday-opts @(ddb/tables :jakso) {:hankkimistapa_id (:hankkimistapa_id jakso)}))) +(defn clear-ddb-tpo-nippu-table! [] + (doseq [tpo-nippu (far/scan @ddb/faraday-opts @(ddb/tables :nippu) {})] + (far/delete-item @ddb/faraday-opts @(ddb/tables :nippu) + {:ohjaaja_ytunnus_kj_tutkinto (:ohjaaja_ytunnus_kj_tutkinto + tpo-nippu) + :niputuspvm (:niputuspvm tpo-nippu)}))) + (def required-jakso-keys #{:ohjaaja_nimi :opiskeluoikeus_oid @@ -338,19 +349,21 @@ :tutkinnonosa_nimi :oppisopimuksen_perusta}) -(deftest test-create-and-save-arvo-vastaajatunnus-for-all-needed! +(deftest test-handle-all-palautteet-waiting-for-vastaajatunnus! (clear-ddb-jakso-table!) + (clear-ddb-tpo-nippu-table!) (testing (str "create-and-save-arvo-vastaajatunnus-for-all-needed! " "calls Arvo, saves vastaajatunnus to db, " "and syncs both palaute and TPO-nippu to heratepalvelu") (with-redefs [organisaatio/get-organisaatio! hoks-utils/mock-get-organisaatio! - koski/get-opiskeluoikeus! hoks-utils/mock-get-opiskeluoikeus! + koski/get-opiskeluoikeus-info-raw + hoks-utils/mock-get-opiskeluoikeus! ;; FIXME: better to have real Arvo fake to test against - arvo/create-jaksotunnus hoks-utils/mock-create-jaksotunnus + arvo/create-jaksotunnus! hoks-utils/mock-create-jaksotunnus date/now #(LocalDate/of 2024 6 30)] (is (= (:status (hoks-utils/create-hoks-in-the-past!)) 200)) - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! {}) + (tep/handle-all-palautteet-waiting-for-vastaajatunnus! {}) (let [palautteet (hoks-utils/palautteet-joissa-vastaajatunnus) ddb-jaksot (far/scan @ddb/faraday-opts @(ddb/tables :jakso) {}) ddb-niput (far/scan @ddb/faraday-opts @(ddb/tables :nippu) {}) @@ -371,48 +384,99 @@ (set (concat required-jakso-keys optional-jakso-keys)))))))))) -(deftest test-create-and-save-arvo-vastaajatunnus-for-all-needed!-error-handling - (testing (str "create-and-save-arvo-vastaajatunnus-for-all-needed! " - "error handling when error occurs in") +(deftest test-handle-palaute-waiting-for-vastaajatunnus!-error-handling + (clear-ddb-jakso-table!) + (clear-ddb-tpo-nippu-table!) + ; Test initialization + (is (= (:status (hoks-utils/create-hoks-in-the-past!)) 200)) + + (let [initial-palautteet (hoks-utils/palautteet) + tep-palaute (nth + (palaute/get-tep-palautteet-waiting-for-vastaajatunnus! + db/spec {:heratepvm (str (date/now))}) 1) + create-jaksotunnus-counter (atom 0) + arvo-tunnukset (atom []) + check-current-state-is-same-as-initial-state + (fn [] + (is (= (hoks-utils/palautteet) initial-palautteet)) + (is (= (ddb/jaksot) [])) + (is (= (ddb/niput) [])) + (is (= @arvo-tunnukset [])))] (with-redefs [organisaatio/get-organisaatio! hoks-utils/mock-get-organisaatio! - koski/get-opiskeluoikeus! hoks-utils/mock-get-opiskeluoikeus! - arvo/create-jaksotunnus hoks-utils/mock-create-jaksotunnus + koski/get-opiskeluoikeus-info-raw + hoks-utils/mock-get-opiskeluoikeus! + arvo/create-jaksotunnus! + (fn [_] (let [tunnus (str (UUID/randomUUID))] + (swap! arvo-tunnukset #(conj % tunnus)) + (swap! create-jaksotunnus-counter inc) + {:tunnus tunnus})) + arvo/delete-jaksotunnus + (fn [tunnus] (swap! arvo-tunnukset #(remove #{tunnus} %))) date/now #(LocalDate/of 2024 6 30)] - (is (= (:status (hoks-utils/create-hoks-in-the-past!)) 200)) - (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5)) - - (testing "Arvo call it should rollback palaute to earlier state" - (with-redefs [arvo/create-jaksotunnus - (fn [_] (throw (ex-info "Arvo error" {})))] - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! {}) - (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5)))) - - (testing (str "DynamoDB sync it should rollback palaute to earlier " - "state and try to delete vastaajatunnus from Arvo") - (let [delete-count (atom 0)] - (with-redefs [tep/sync-jakso-to-heratepalvelu! - (fn [_ __ ___ ____ _____] - (throw (ex-info "DDB sync error" {}))) - arvo/delete-jaksotunnus - (fn [tunnus] - (assert tunnus) - (swap! delete-count inc))] - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! {}) - (is (= @delete-count 5)) - (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5))))) - - (testing (str "getting opiskeluoikeus from Koski (not found) it " - "should skip getting vastaajatunnus for that " - "jakso/hoks") - (with-redefs [koski/get-opiskeluoikeus! (fn [_] nil)] - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! {}) - (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5)))) - (testing (str "getting opiskeluoikeus from Koski (other http error) " - "it should skip getting vastaajatunnus for that " - "jakso/hoks") - (with-redefs [koski/get-opiskeluoikeus! - (fn [oid] - (throw (ex-info "Koski error" {:status 500})))] - (tep/create-and-save-arvo-vastaajatunnus-for-all-needed! {}) - (is (= (count (hoks-utils/kasittelemattomat-palauteet)) 5))))))) + (testing (str "Everything (palaute-backend DB and DynamoDB tables) rolls " + "back to its initial state (before the function call) when") + (testing "Arvo call for vastaajatunnus creation fails." + (with-redefs [arvo/create-jaksotunnus! + (fn [_] (throw (ex-info "Arvo error" {})))] + (is (thrown-with-msg? + ExceptionInfo + #"Arvo error" + (tep/handle-palaute-waiting-for-vastaajatunnus! + tep-palaute)))) + (check-current-state-is-same-as-initial-state)) + (testing "jakso sync to Herätepalvelu fails." + (with-redefs [heratepalvelu/sync-jakso!* + (fn [_] (throw (ex-info "Jakso sync error" {})))] + (is (thrown-with-msg? + ExceptionInfo + #"Failed to sync jakso" + (tep/handle-palaute-waiting-for-vastaajatunnus! + tep-palaute)))) + (check-current-state-is-same-as-initial-state)) + (testing "nippu sync to Herätepalvelu fails." + (with-redefs + [heratepalvelu/sync-tpo-nippu!* + (fn [_] (throw (ex-info "TPO-nippu sync failed" {})))] + (is (thrown-with-msg? + ExceptionInfo + #"Failed to sync TPO-nippu" + (tep/handle-palaute-waiting-for-vastaajatunnus! + tep-palaute)))) + (check-current-state-is-same-as-initial-state))) + (let [counter-value-before-fn-call @create-jaksotunnus-counter] + (testing (str "When getting other than \"404 Not found\" error from " + "Koski the function should skip palaute processing. " + "No call to Arvo should be made.") + (with-redefs + [koski/get-opiskeluoikeus-info-raw + (fn [oid] + (throw (ex-info + "Koski error" + {:status 500 + :type ::koski/opiskeluoikeus-fetching-error})))] + (is (thrown-with-msg? + ExceptionInfo + #"Error while fetching opiskeluoikeus" + (tep/handle-palaute-waiting-for-vastaajatunnus! tep-palaute))) + (is (= counter-value-before-fn-call @create-jaksotunnus-counter))) + (check-current-state-is-same-as-initial-state)) + (testing (str "When opiskeluoikeus for palaute is not found from from " + "Koski, `tila` for it should be marked as `ei_laheteta`. " + "No call to Arvo should be made.") + (with-redefs [koski/get-opiskeluoikeus! (fn [_] nil)] + (tep/handle-palaute-waiting-for-vastaajatunnus! tep-palaute) + (is (= counter-value-before-fn-call @create-jaksotunnus-counter)) + (is (= (:tila (palaute/get-by-id! db/spec {:id (:id tep-palaute)})) + "ei_laheteta")))) + (testing (str "Palaute should be marked as \"ei_laheteta\" when there " + "are one or more open keskeytymisajanjakso. No call to " + "Arvo should be made.") + (with-redefs [oht/get-keskeytymisajanjaksot! + (fn [_ __] [{:alku (LocalDate/of 2023 11 1) + :loppu (LocalDate/of 2023 11 16)} + {:alku (LocalDate/of 2024 02 5)}])] + (tep/handle-palaute-waiting-for-vastaajatunnus! tep-palaute) + (is (= counter-value-before-fn-call @create-jaksotunnus-counter)) + (is (= (:tila (palaute/get-by-id! db/spec {:id (:id tep-palaute)})) + "ei_laheteta"))))))))