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
Slice: Opiskelijan tietoihin uusia kenttia #1549
Slice: Opiskelijan tietoihin uusia kenttia #1549
Conversation
…ijan_tietoihin_uusia_kenttia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paljon pohdintaa ja jotain korjattavaa
persistence/src/main/java/fi/otavanopisto/pyramus/domainmodel/students/StudentCard.java
Outdated
Show resolved
Hide resolved
persistence/src/main/java/fi/otavanopisto/pyramus/domainmodel/students/StudentCard.java
Outdated
Show resolved
Hide resolved
persistence/src/main/java/fi/otavanopisto/pyramus/domainmodel/students/StudentCard.java
Show resolved
Hide resolved
persistence/src/main/java/fi/otavanopisto/pyramus/domainmodel/students/StudentCardType.java
Outdated
Show resolved
Hide resolved
persistence/src/main/java/fi/otavanopisto/pyramus/dao/students/StudentCardDAO.java
Outdated
Show resolved
Hide resolved
...us/src/main/java/fi/otavanopisto/pyramus/json/students/EditStudentJSONRequestController.java
Outdated
Show resolved
Hide resolved
Date expiryDate = null; | ||
|
||
// Set expiry date automatically same as study end date or study time end | ||
if (student.getStudyEndDate() != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tämä voisi olla ihan validi lähestymistapa, en nyt äkkisältään osaa ajatella miten kaikki tulkinnat tulisi toimimaan, jos päättymispäivä tallennettaisiin näin.
Itse ajattelin alunperin, että päättely tehtäisiin vasta listaamisvaiheessa, jolloin tässä tallennettaisiin aina vain tämä "kovakoodattu" aikaraja. Sen huono puoli on siinä, että tulkinta tulee olemaan vähän hankalaa, mutta hyvä puoli siinä, että nämä eri päivämäärät olisi aina erikseen tallessa. Mutta joo, en nyt osaa ajatella kaikkia vaikutuksia tähän päättelylogiikkaan niin voinee olla näinkin.. :D
pyramus/src/main/resources/fi/otavanopisto/pyramus/I18N/pyramuslocale_fi_FI.properties
Outdated
Show resolved
Hide resolved
...us/src/main/java/fi/otavanopisto/pyramus/json/students/EditStudentJSONRequestController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Foreign keyn skripti ja pientä säätöä
active bit not null, | ||
expiryDate date not null, | ||
type varchar(255) not null, | ||
student_id bigint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tältä puuttuu foreign key, generoidusta sql:stä se löytyy erillisenä lauseena - taulun nimellä etsimällä pitäisi löytyä kaikki siihen tauluun kohdistuvat skriptit.
return studentCard; | ||
} | ||
|
||
public StudentCard findByStudent(Long studentId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vaihtaisin Long studentId
:n Student student
:ksi, jolloin on aina selvää, mitä parametria odotetaan (Long:ja kun löytyy kaikista id-kentistä).
En ole ihan varma toimiiko tuo where-ehdossa oleva
criteriaBuilder.equal(root.get(StudentCard_.student), studentId)
(StudentCard_.student on tyyppiä Student, studentId tyyppiä Long) mutta jos parametrityypin muuttaa, niin
criteriaBuilder.equal(root.get(StudentCard_.student), student)
ainakin pitäisi toimia (molemmat tyyppiä Student).
@@ -1810,10 +1825,10 @@ students.editStudent.studyPeriodsTable.removeTooltip = Poista t | |||
students.editStudent.addStudyPeriodLink = Lis�� opiskelujakso | |||
students.editStudent.selectUserVariablePresetDialog.title = Valitse esiasetettu muuttujan arvo | |||
students.editStudent.selectUserVariablePresetDialog.tabLabel = Esiasetukset | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tämän tyhjän rivin voisi lisätä takaisin jotta rivinumerot mätsää toisen lokaalin kanssa :D (taitaa olla rivi 1828 tässä branchissä)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor optimization and readability changes.
...us/src/main/java/fi/otavanopisto/pyramus/json/students/EditStudentJSONRequestController.java
Outdated
Show resolved
Hide resolved
…ijan_tietoihin_uusia_kenttia
closes #1541