Fixing error template #43
Conversation
…ll using two different .ft. I want to change this)
There was a problem hiding this comment.
Good start and the basic idea should enable us to render partial templates. But of course I have a few remarks :)
But also in the templates we need to verify that really only the relevant part is returned. It can be useful to have a look at the server responses in the Inspector in your browser.
| @@ -18,6 +18,7 @@ import net.grandcentrix.backend.repository.HousesRepository.Companion.HousesRepo | |||
| import net.grandcentrix.backend.repository.MoviesRepository.Companion.MoviesRepositoryInstance | |||
| import net.grandcentrix.backend.repository.PotionsRepository.Companion.PotionsRepositoryInstance | |||
| import net.grandcentrix.backend.repository.SpellsRepository.Companion.SpellsRepositoryInstance | |||
| import respondTemplates | |||
There was a problem hiding this comment.
That's a weird import. Usually imports are a reverse FQDN of some kind.
There was a problem hiding this comment.
intelij just told me to do this
| } | ||
|
|
||
| suspend fun ApplicationCall.respondTemplates( |
There was a problem hiding this comment.
Why the plural in the name here? We must only respond with a single response and this response is rendered based on one template. This template might reference other templates as a form of composition, but it is still a single template.
As we already discussed we unfortunately can't overwrite already existing extension functions easily, but better naming is always a good idea :)
| suspend fun ApplicationCall.respondTemplates( | ||
| template: String, // The name of the template to be rendered | ||
| model: Any? = null, // The data model to be passed to the template, default is null |
There was a problem hiding this comment.
Do we have cases where the model is null?
| template: String, // The name of the template to be rendered | ||
| model: Any? = null, // The data model to be passed to the template, default is null | ||
| etag: String? = null, // The ETag for caching purposes, default is null |
There was a problem hiding this comment.
Do we ever set the etag when calling respondTemplates?
| model: Any? = null, // The data model to be passed to the template, default is null | ||
| etag: String? = null, // The ETag for caching purposes, default is null | ||
| contentType: ContentType = ContentType.Text.Html.withCharset(Charsets.UTF_8) // The content type of the response, default is HTML with UTF-8 charset |
There was a problem hiding this comment.
Nice to set the correct content type, but do we need this as a parameter? The templates will always be HTML. If we want a different content format we most likely need to create another mechanism to build our responses.
There was a problem hiding this comment.
okay as we talked about i just copy/pasted the implementation because it was marked as "Read only" so i didnt thought about it much more. But i will have a look at this and the two things above and see if it breaks something when i delete it or change it
| val modelWithHxRequest = if (model is Map<*, *>) { | ||
| model + ("hxRequest" to hxRequest) // Add hxRequest to the existing model map | ||
| } else { |
There was a problem hiding this comment.
That could cause problems. In your parameters you accept Any? as a type for the model. And here you check if model is not just Any but specifically some kind of Map. But what happens here if we have a model which does not implement the Map interface? Would a template depending on such a model render correctly?
…d mapping works to make it better
No description provided.