Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

Implement APIs for palveluväylä #1043

Merged
merged 6 commits into from
Nov 27, 2023
Merged

Conversation

solita-antti-mottonen
Copy link
Contributor

No description provided.

@solita-antti-mottonen solita-antti-mottonen force-pushed the new-apis-tryout branch 2 times, most recently from a1640e5 to 3af4e71 Compare November 22, 2023 10:30
LibreOffice prints out warning to stderr on newer MacOS versions. The
warning doesn't seem to be anything serious, and there doesn't seem to
be any solutions for it available. Exit code is still checked for
errors.
{{:keys [accept-language]} :header} :parameters
{{:keys [id]} :path} :parameters}]
(let [language-preference-order (if accept-language
(->> accept-language
Copy link
Contributor

Choose a reason for hiding this comment

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

Tämä voisi olla oma funktionsa

(map first))
["fi" "sv"])]
(api-response/pdf-response ; Return the first language version that exists if any
(some identity (->> language-preference-order
Copy link
Contributor

Choose a reason for hiding this comment

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

Olisiko tämä kieliversion päättely parempi olla service-kerroksessa?

(->> (energiatodistus-service/file-key id kieli)
(file-service/find-file aws-s3-client)
io/input-stream))

(defn find-energiatodistus-pdf [db aws-s3-client whoami id kieli]
(when-let [{:keys [allekirjoitusaika] :as complete-energiatodistus}
(complete-energiatodistus-service/find-complete-energiatodistus db whoami id)]
(-> (complete-energiatodistus-service/find-complete-energiatodistus db whoami id)
(#(when (or (= (-> % :perustiedot :kieli) 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tätä voisi selkiyttää jotenkin

(def palveluvayla-basepath "/api/palveluvayla/energiatodistukset")

(t/deftest test-palveluvayla-api
(let [laatija-id (first (keys (test-data.laatija/generate-and-insert! 1)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Tähän testisetupiin voisi kommenteilla kertoa, että mitä tässä oikein tehdään, koska tätä on niin paljon ja hälyisän näkyistä koodia, että siitä ei pikavilkaisulla oikein ymmärrä mitä tapahtuu.

(service.energiatodistus/find-energiatodistus-any-laatija db id))
([id db version]
(-> (service.energiatodistus/find-energiatodistus db id)
(#(drop-wrong-version % version)))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Tarvitseeko tässä enää tätä anonyymiin funktioon käärimistä?

"Return the first language version that exists if any. If language-preference-order is not given, [fi sv] is used."
[id language-preference-order db aws-s3-client]
(some identity (->> (or language-preference-order ["fi" "sv"])
(map #(service.energiatodistus-pdf/find-energiatodistus-pdf db
Copy link
Contributor

@Juholei Juholei Nov 23, 2023

Choose a reason for hiding this comment

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

Tässä voisi ehkä tuon mapin korvata jollakin ei-laiskalla funktiolla (ihan vain mapv?), jotta energiatodistuksen haku blob storagesta tapahtuu varmasti ennakoitavaan aikaan koodissa.

(some identity) voisi myös olla threading-makrossa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideana oli hakea niistä vain eka joka löytyy. Clojurehan ei niin tee, mutta ajattelin että kommunikoi paremmin haetun toiminnallisuuden jos niin tekee.

(some identity) toimii myö makrossa, mutta taas minusta näin päin tuntuu kommunikoivan tarkoituksen paremmin.

Create a service for Palveluväylä to make the codebase a bit clearer.
Separate parsing languages into a function. Clean up the test code a
bit. Comment language discrimination in pdf search.
(t/is (= 200 (:status response)))
(t/is (= todistus-2013-fi-id (:id body)))
(schema-tools.coerce/coerce body schema.energiatodistus/EnergiatodistusForAnyLaatija schema-tools.coerce/json-coercion-matcher)))
(t/testing "Fetching energiatodistus by id returns it when fetching correct version"
Copy link
Contributor

Choose a reason for hiding this comment

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

Laita näiden testing-lohkojen väliin tyhjät rivit, niin erottaa paremmin missä yksi testicase päättyy ja toinen alkaa.

body (j/read-value (:body response) j/keyword-keys-object-mapper)]
(t/is (= 200 (:status response)))
(t/is (= 6 (count body)))
(t/is (= #{todistus-2013-fi-id todistus-2018-fi-id todistus-2013-sv-id todistus-2018-sv-id todistus-2013-multilingual-id todistus-2018-multilingual-id} (set (map :id body))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Laittaisin näin monen alkion setissä jokaisen omalle rivilleen kuten myös tuon mihin sitä testissä vertaillaan. Erottuu sitten paremmin myös se mitä tähän vertaillaan.

@solita-antti-mottonen solita-antti-mottonen merged commit 82cb262 into develop Nov 27, 2023
4 checks passed
@solita-antti-mottonen solita-antti-mottonen deleted the new-apis-tryout branch November 27, 2023 08:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants