Conversation
|
Diese Logik ist auch für die Nutzerliste relevant und ich finde, dass sie in |
antoniaammon
changed the title
|
Hab jetzt was in JTE context gemacht, ist das so okay? @JFronny |
JFronny
left a comment
JFronny
left a comment
There was a problem hiding this comment.
Not quite happy with the implementation here, but the basic idea is definitely good
src/main/jte/layout.jte
Outdated
| ? context.lookup("user.visitor") | ||
| : layout.getLogin().name())}</label> | ||
| <label> | ||
| ${context.localize("layout.welcome", context.getUserName(layout.getLogin()))} |
There was a problem hiding this comment.
This will cause a NPE if the user is not logged in.
Please handle the case of user == null in getUserName
There was a problem hiding this comment.
Aber das ist doch die Variante, die das LoginStateModel und nicht den User nutzt, und das ist immer vorhanden oder?
| return new PageSpec(lookup(key), "SOLI"); | ||
| } | ||
|
|
||
| public String getUserName(LoginStateModel login) { |
There was a problem hiding this comment.
This should be named format and return Content
There was a problem hiding this comment.
Wie mach ich dann das login.name() zu Content?
| } | ||
| } | ||
|
|
||
| public String getUserName(User user) { |
There was a problem hiding this comment.
This should be named format and return Content
| } | ||
|
|
||
| public String getUserName(LoginStateModel login) { | ||
| switch (login.kind()) { |
There was a problem hiding this comment.
Please use a switch expression, not a statement
There was a problem hiding this comment.
Wo ist der Unterschied?
There was a problem hiding this comment.
Die curly braces sind unnötig und du kannst return rausziehen. Ich fixe es.
| } | ||
|
|
||
| public String getUserName(User user) { | ||
| if (Objects.equals(user.getUsername(), "Guest")) { |
There was a problem hiding this comment.
This should be based on the implementation of this check in UserService, not an ad-hoc check using the username. The service can be injected indirectly through the constructor.
Also, admin users should be handled.
…ame-localize # Conflicts: # src/main/resources/messages.properties # src/main/resources/messages_de.properties
github-project-automation
bot
moved this from In Review
to Done
in Solidarische Raumnutzung
2 participants
ich dachte dass das so am einfachsten ist oder ist das hässlich?
closes #208