Calling mark_intro_complete doesn't respond correctly

[Huw Wilkins]

When calling mark_intro_complete (https://github.com/maas/maas/blob/e70cca2fb81cf248ad1f080adade478973a7bfe4/src/maasserver/websockets/handlers/user.py#L194) there are two issues:

1. The API sends a response to `mark_intro_complete` that includes an out of date user (at the very least `sshkeys_count` is sent as `0` instead of the correct number).

2. The API does not send a `user.update` notify message to tell the client that the user data was changed.

[Bill Wear (billwear)]

not able to test this ATM. leaving in New state.

[Björn Tillenius (bjornt)]

Can you include the steps that you take in the UI to reproduce this?

I think I see a problem, though. UserHandler.mark_intro_completed uses self.user, which might be out-of-date if you did operations, like adding an SSH key to the user.

[Huw Wilkins (huwshimi)]

To reproduce this:
- create a new user
- log in as that new user
- you should see the user intro
- open the network tab of your devtools and click on the messages list for the websocket (you may wish to clear the list so you can spot new messages)
- add an SSH key
- you should get a websocket user update message with should have `sshkeys_count: 1`
- click "Finish setup"
- you should see a message for user.mark_intro_complete and in the response message you should see `sshkeys_count: 0`

[Björn Tillenius (bjornt)]

Ok, thanks. That confirms my suspicion. The UserHandler shouldn't operate on self.user directly. It needs to get an up-to-date object from the db.

[Alberto Donato (ack)]

The issue is that we currently cache sshkeys_count and machines_count for the user on the self.user object itself.

Since the object lives for the duration of the websocket connection, we need to keep the cache up to date.
For sshkeys we have to do that on keys creation, import and deletion. I suspect a similar issue might happen with machines count (which is harder to keep up to date since there's multiple places where the machine can be acquired/released).

I wonder if the UI really needs to always have these counts available. If they're only needed in specific cases, we could just fetch those instead of putting them on the user.

[Huw Wilkins (huwshimi)]

It looks like we only use user.machines_count in the user list in settings and user.sshkeys_count in the user list and machine deploy form.

We could fetch machines_count and sshkeys_count in those places but we'd need to be able to fetch them both for the authenticated user as well as for all users (in one call).

[Alberto Donato (ack)]

For the user listing page, it should be easy to add them.

But why is the ssh keys count needed for the machine deploy form?

[Alberto Donato (ack)]

Caching is also made tricky by the fact that we have 3 methods returning the state for the current user: user.auth_user, user.change_password and user.mark_intro_complete.

It'd be good if we could change things so that just the first one returns the user, others just perform changes.

The UI can call user.auth_user to get up to date info about the current user, and user.list to get all the users. In both cases we'd include the counts for machines and ssh keys.

[Huw Wilkins (huwshimi)]

> But why is the ssh keys count needed for the machine deploy form?

It uses the ssh keys count to show a warning if the user has no keys: "Login will not be possible because no SSH keys have been added to your account. To add an SSH key, visit your account page."

> Caching is also made tricky by the fact that we have 3 methods returning the state for the current user: user.auth_user, user.change_password and user.mark_intro_complete.
>
> It'd be good if we could change things so that just the first one returns the user, others just perform changes.

That would work for us. If we get notify messages when the auth user changes then we wouldn't need to do anything with the responses from change_password and mark_intro_complete.

> The UI can call user.auth_user to get up to date info about the current user, and user.list to get all the users. In both cases we'd include the counts for machines and ssh keys.

This would be fine for initially getting the data, but we'd still need the notify messages for when the machine/ssh key counts change.