-
Notifications
You must be signed in to change notification settings - Fork 21
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
Other entity login #2143
Other entity login #2143
Conversation
Deployed to https://pr-2143.aam-digital.net/ |
src/app/core/common-components/entity-subrecord/list-paginator/list-paginator.component.ts
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.
Code looks nice 👍
I guess this required the getId() PR to be merged first?
Tested
- pagination
- todo completion
not tested
- user security component with Keycloak BE
- todo completion with non-user user entity
- Profile (menu bottom) shows an error in demo mode:
undefined (reading 'loadUserInfo')
- demo mode SecurityComponent still queries keycloak (and fails) - should we prevent that?
- complete todo shows error in details popup and related-todos list:
Cannot read properties of undefined (reading 'toString')
...app/core/basic-datatypes/entity-array/display-entity-array/display-entity-array.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts
Show resolved
Hide resolved
src/app/core/common-components/entity-select/entity-select.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/common-components/entity-form/entity-form.service.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # src/app/core/basic-datatypes/entity/display-entity/display-entity.component.ts # src/app/core/common-components/entity-form/entity-form.service.spec.ts # src/app/core/common-components/entity-select/entity-select.component.spec.ts # src/app/core/common-components/entity-select/entity-select.component.ts # src/app/core/entity/default-datatype/view.directive.ts # src/app/core/session/session-service/session-manager.service.ts # src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.html # src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.spec.ts # src/app/features/todos/todo-completion/display-todo-completion/display-todo-completion.component.ts
@sleidig I did not change anything about the failing Keycloak requests in the demo mode yet. I am not sure what we actually want there. Someone added the account manager role to the demo admin user which enables the component. If we just remove this role, the errors disappear. Or do we want to showcase something there in the demo mode? |
I thought we've linked the user list to that role, but apparently it is using admin_app. So what you are saying makes sense - can't think of a need for account_manager in demo mode. |
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.
Further Testing:
- user profile (bottom left):
- the username field gets overwritten by my Lastpass - although I guess it is disabled? (maybe something for another day ...)
- can we add a link to the Entity the "User profile" (i.e. entity) this is represented as in our system? (maybe simply with display-entity? / edit-entity)
- creating new user accounts:
- the email template says "Your email has been linked to the Aam Digital user: User:Aadi". This will for most cases now be very cryptic - and otherwise might confuse people about what to put into the login form. Shall we take out that sentence and just say "An account for the Aam Digital system has been created for your email. By confirming your account below, you can set a password and then log in using this email as your username."?
- new user is linked and using correct entity to prefill notes
- config the UserSecurity component for child details and creating user account from there works
- tasks & notes (with pre-filled current user's entity)
- works for notes + tasks with User and Child entity, as expected
- works without errors if no entity for the user account
- task completion works with any entity type
: await this.entityMapperService.load<E>(type, id); | ||
|
||
if (!entity) { | ||
throw Error(`Entity not found`); |
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.
throw Error(`Entity not found`); | |
throw Error(`Entity ${id} not found`); |
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.
I have added the ID logging to the surrounding function.
@sleidig For LastPass there are two options. I recommend checking "Don't overwrite fields that are already filled" ("Account" > "Extensions Settings" > "General" at the bottom), which will also disable this at other locations (e.g. Keycloak UI). |
…pec.ts Co-authored-by: Sebastian <sebastian@aam-digital.com>
046e94d
to
411e2e3
Compare
411e2e3
to
211e229
Compare
@sleidig Thanks for the review. |
🎉 This PR is included in version 3.32.0-master.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 3.32.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
closes: #2132 #1685 #1513
UserSecurityComponent
is entity agnostic$currentUser
default value -> assign full entity ID and support "foreign" entity types in display/edit entity components -> This comes with the limitation that our default filter (e.g. notes of child) will not include this "foreign" references (e.g. when a child has been added as author of a note)getId()
function #1526 and skip this)username
(exact_username
attribute in keycloak) access token claim optional and use user ID instead for local DB namesexact_username
) claim for offline user selection