fix: using initial username as id and preventing later edits #1503
Conversation
|
Deployed to https://test-deployment-pr-1503.herokuapp.com/ |
| // Throwing error if trying to change existing username | ||
| const label = User.schema.get("name").label; | ||
| throw new Error( | ||
| $localize`:Error message when trying to change the username|e.g. username cannot be changed after initialization:${label} cannot be changed after initialization` |
There was a problem hiding this comment.
best practices would suggest to not mix logic (the Entity model here) with UI (translating an error message), I think. The clean but more complex approach would be to define error codes or classes and only put human-readable and translated error messages when displaying it to the user. But I have my doubts that complexity is a worthy trade off here. 👍
There was a problem hiding this comment.
Definitely a good thought. At the moment we are missing a general display error approach where we could translate these errors to a user friendly name.
| @@ -34,7 +34,23 @@ export class User extends Entity { | |||
| label: $localize`:Label of username:Username`, | |||
| validators: { required: true }, | |||
| }) | |||
| name: string; | |||
| set name(value: string) { | |||
There was a problem hiding this comment.
let's maybe change the property to username (or will this break dozens of things?). For some clients we introduced additional firstname and lastname properties to be used in the UI. "username" would make the purpose of this one more explicit
There was a problem hiding this comment.
At the moment it would break a dozens of things (all the current user entities) So lets get this out and if we ever want to migrate this, we can do this independently.
|
Kudos, SonarCloud Quality Gate passed!
|
|
🎉 This PR is included in version 3.13.2-master.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.13.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
When creating new user they currently get a UUID (like all other entities). The problem is we need the user's id to match the username of the Keycloak user. This is important in order to assign users to entities and then later match them with actually logged in users.
This approach initially overwrites the entity id when the first time a username is saved but afterwards does not allow to change the username anymore. This way we ensure that user entities can be matched with the usernames.
Visible/Frontend Changes
--
Architectural/Backend Changes
Userentity, the name is used asentityId