Simplify error handling in handle-palaute-waiting-for-vastaajatunnus!

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))))

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.

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) {}))

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))

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)))))))
 

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)}))

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" []