Conversation
gzdunek
added
the
no-changelog
gzdunek
force-pushed
the
gzdunek/user-preferences-handlers
branch
from
5887ba3 to
d42490e
Compare
|
In the web, there is a this notion of "global" user preferences and then "cluster" user preferences. The difference is mostly one always pulls from the root cluster, and the other pulls from whatever cluster is being requested (root or leaf or whatever). Is there a distinction here? |
|
Yeah, I'm aware of that distinction (view mode and active tab are always taken from the root cluster, while pinned resources are taken from the target root or leaf cluster). But it seemed to me that it should be transparent at the tshd level and handled in a JS service (which will be done in the upcoming PR). |
| updateUserPreferences( | ||
| params: api.UpdateUserPreferencesRequest.AsObject, | ||
| abortSignal?: types.TshAbortSignal | ||
| ): Promise<void> { |
There was a problem hiding this comment.
The way
authClient.UpdateUserPreferences()work is that it expects a full preferences message. If any field is missing, it will be reset to the default value. Since we work on a subset in tshd, I need to fetch the previous preferences first and modify only the changed fields. This may seem a bit ineffective, but as I mentioned above, the only other way is to work on full preferences messages.
Where do you plan to do this? In the client or in tshd?
There was a problem hiding this comment.
Where do you plan to do this? In the client or in tshd?
I did it in tshd.
…nd leaf cluster preferences, return updated preferences after changing them on a server
ravicious
left a comment
ravicious
left a comment
There was a problem hiding this comment.
I haven't ran the new code for userpreferences but it looks okay!
| preferences, err := leafClient.GetUserPreferences(ctx, &userpreferencesv1.GetUserPreferencesRequest{}) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } |
There was a problem hiding this comment.
this conditional should only overwrite cluster prefs and not userprefs. preferences held in UserPreferences (from the root) are the settings we don't want to change like selected theme and what not
There was a problem hiding this comment.
Agree, but I think we are only overwriting the cluster preferences, no?
There was a problem hiding this comment.
Oh yes, my bad. Read it wrong
|
Added support for |
3 participants
Contributes to #30422
Frontend part: #35251
This PR adds to new handlers to tshd:
GetUserPreferencesUpdateUserPreferencesWe will need to them to fetch and update pinned resources and the active tab in the unified view.
The most important things:
teleport/userpreferences. They work with no changes in Go code, but for JS I had to generate them for the first time. I decided to generate the JS code only forapi/proto/teleport/userpreferences/since this is the only part we need.UserPreferencesmessage that contains only the fields that are relevant to Connect. My reasoning was that we don't needthemeoronboardingparams. Also, if I used the full preferences message, on the JS side I would have (and anyone else who adds a new field) to map all these fields to a gRPC message increateClient.updateUserPreferences().The main problem with that is if someone else adds a new field to the user preferences, there won't be any error if that field is not mapped the gRPC message in JS.
authClient.UpdateUserPreferences()work is that it expects a full preferences message. If any field is missing, it will be reset to the default value. Since we work on a subset in tshd, I need to fetch the previous preferences first and modify only the changed fields. This may seem a bit ineffective, but as I mentioned above, the only other way is to work on full preferences messages.I will connect these APIs with the unified resources view in a separate PR.