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.

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

Ok 345 #1455

Merged
merged 134 commits into from
Nov 23, 2023
Merged

Ok 345 #1455

merged 134 commits into from
Nov 23, 2023

Conversation

msiukola
Copy link
Contributor

@msiukola msiukola commented Sep 7, 2023

No description provided.

@@ -45,7 +45,8 @@
:attachment-file-part-max-size-bytes 5242880
:job-failure-alert-recipients "testi@example.org"
:send-job-failure-alert-emails true
:features {:schema-validation true}}
:features {:schema-validation true
Copy link
Contributor

Choose a reason for hiding this comment

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

Sivuhuomiona, mut mietin pitäiskö tosta schema-validationista luopua? Sehän taisi olla se kikkare mikä varmistaa ja kuormittaa kälissä niitä lomakkeen vastauksia.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Toi taitaa ympäristöissä (ainakin QA ja tuotanto) ollakin poissa päältä, oiskohan se enää noissa devikonffeissa päällä

config/dev.edn Outdated Show resolved Hide resolved

.application__hakeminen-tunnustautuneena-partial-line-left {
width: 45%;
float: left;
Copy link
Contributor

@SalamaGofore SalamaGofore Sep 8, 2023

Choose a reason for hiding this comment

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

En ole nähnyt käli kuvia, mut mieluummin kannattaa käyttää flexiä tai gridiäkin floatin sijaan yleisesti.

@@ -140,6 +145,86 @@
(defn- not-blank? [x]
(not (clojure.string/blank? x)))

(defn parse-oppija-attributes-if-successful [validation-response]
Copy link
Contributor

Choose a reason for hiding this comment

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

olisko tätä varten hyvä käyttää jotain schemaa hakijan tiedoille?

:query-params [{ticket :- s/Str nil}
{target :- s/Str nil}
{lang :- s/Str nil}]
;fixme ehkä, pitäisikö myös tarkistaa onko jo voimassaoleva sessio?
Copy link
Contributor

Choose a reason for hiding this comment

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

Riippuu näytetäänkö tunnistautuneelle hakijalle kirjautumisvaihtoehtoa (ei mielestäni pitäisi)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ei näytetä, mut ehkä rajapinnan toimintaa kannattaa miettiä siinäkin tilanteessa, jos vaikka joku erehtyy sitä jostain kutsumaan. Oiskohan suora ohjaus target-urliin järkevä siinä tapauksessa?

(oss/read-session)))]
(log/info "Submit application, tunnistautunut" tunnistautunut? ", session" oppija-session-from-db)
(if (and tunnistautunut? (nil? oppija-session-from-db))
(response/bad-request {:passed? false :failures ["Sessio on vanhentunut"] :code :session-not-found})
Copy link
Contributor

Choose a reason for hiding this comment

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

Onko keissiä että lomakkeella pitää olla tunnistautunut? Lähinnä jos ei, niin mietin tarviiko teoriassa vapaasti täydennettävien tietojen takia estää hakemusta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Toi on hyvä huomio kyllä. Toisaalta pitää jotenkin hanskata se keissi, että hakija luulee olevansa tunnistautunut ja onkin ollut, mutta hakemusta on täytetty kauan ja lähettäessä selviää että sessio ei ole enää voimassa. Pakkohan ei ole tunnistautua, mutta jos tuo tunnistautunut? -flagi on päällä niin kälin mielestä hakija on halunnut tunnistautua.

@@ -94,23 +95,52 @@
(when form
[editable-fields form submit-status])))))

(defn- hakeminen-tunnistautuneena-lander []
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ä komponentti voisi olla omassa filussaan, mietin pitäisikö olla myös oma less-filu tän tyyleille

@msiukola msiukola marked this pull request as ready for review November 23, 2023 12:31
@msiukola msiukola merged commit e773f6b into master Nov 23, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants