Feature - GET /questionnaires#2
Conversation
CyberAve2100
requested review from
DaanTol,
RamonKea-inh,
SemPlaatsman and
Tayam-El-Jebari
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
SemPlaatsman
left a comment
SemPlaatsman
left a comment
There was a problem hiding this comment.
Basically Daan had laatst nog wat nieuwe db connection code geschreven. Als je dat overneemt hoef je verder geen mapping of omweg te verzinnen voor het laten werken in openshift, dan kan je het gelijk hosten, wat super chill is.
| POSTGRESQL_DB_URL= | ||
| POSTGRESQL_DB_USERNAME= | ||
| POSTGRESQL_DB_PASSWORD= No newline at end of file |
There was a problem hiding this comment.
Check de nieuwste code van Daan, heeft nu namelijk andere env variables. Die heeft er nu namelijk 6 ofzo en die structuur moet je overnemen als je wilt dat je db werkt in openshift. Kijk ook naar zijn application.properties
| # Database Configuration | ||
| azure.storage.connection-string=${AZURE_STORAGE_CONNECTION_STRING:UseDevelopmentStorage=true} | ||
| azure.storage.table-name=${AZURE_STORAGE_TABLE_NAME:products} | ||
| azure.storage.table-name=${AZURE_STORAGE_TABLE_NAME:products} No newline at end of file |
There was a problem hiding this comment.
Hier ook nog even naar kijken en kopiëren wat Daan heeft
| <trimTrailingWhitespace /> | ||
| <endWithNewline /> |
| val title: String, | ||
| val description: String, | ||
| val estimatedTimeOfCompletion: String, | ||
| val assignedAt: String, |
There was a problem hiding this comment.
why is assigned at a string? Should be date time no?
There was a problem hiding this comment.
In the swagger it is annotated this way so the response should give back a string right?
There was a problem hiding this comment.
One question, assignedAt is not confused with assignedTo right? Cause does the assignedAt field mean the date at which the questionnaire was assigned, or who it was assigned to? Because if it's the date at which the questionnaire was assigned, datetime is more logical, but if it's confused with who it was assigned to, then it should be string (and maybe a more clear name, e.g. assignedUserId or something).
PS: Something seems off in Swagger, always double check.
| var id: String = "" | ||
| var title: String = "" | ||
| var description: String = "" | ||
| var estimatedTimeOfCompletion: String = "" | ||
| var assignedAt: String = "" |
There was a problem hiding this comment.
these default values are redundant, as it automatically becomes an empty string
There was a problem hiding this comment.
why do you need this to be vars btw? can't you just make this a data class?
data class Questionnaire(
val id: String,
val title: String,
val description: String,
val estimatedTimeOfCompletion: String,
val assignedAt: String,
val isFinished: Boolean
)
There was a problem hiding this comment.
true, changed it
4 participants
Not fully tested yet but putting it up for feedback already, it runs for me locally but cant test atm because the local db isnt configured yet