Skip to content
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

Updating displayName Reactive #2994 #3403

Merged
merged 2 commits into from
Apr 8, 2023

Conversation

DeNic0la
Copy link
Collaborator

@DeNic0la DeNic0la commented Apr 7, 2023

PR for issue #2994

Reload user entity after changing firstname, surname or nickname

@manuelmeister
Copy link
Member

So cool. Thank you for your contribution. I'll review it tomorrow 😊

@carlobeltrame carlobeltrame added the deploy! Creates a feature branch deployment for this PR label Apr 7, 2023
@github-actions
Copy link

github-actions bot commented Apr 7, 2023

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

methods: {
reloadUser() {
this.api.reload(this.user).then((user) => {
this.$store.commit('login', user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this more easily understandable, could you add another mutation called "updateUser" to
frontend/src/components/store/auth.js, and then use that here?
The mutation code (single line) will be duplicated, but what we're doing here isn't really a re-login, but an update of the user state. Maybe these two actions will have different behaviour in the future.

@carlobeltrame
Copy link
Member

carlobeltrame commented Apr 7, 2023

I also noticed the update is not applied if I navigate away from the profile edit during the save operation (I was testing on my phone). Not sure how to solve this though...

@carlobeltrame
Copy link
Member

carlobeltrame commented Apr 7, 2023

Maybe it was also a bad idea of us in the first place, to store a duplicated user state in the auth store. Maybe we would be better off storing only the user URI (which we get from the JWT from the API) there, and reading the user data from the normal API store when needed. But this auth.user is used in too many places so you don't have to refactor this in this PR.

Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@pmattmann
Copy link
Member

@DeNic0la: Very cool - thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants