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.

Sign up for GitHub

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

[papi] Add UserService/UpdateUser to proto #19163

Merged
merged 2 commits into from
Nov 29, 2023
Merged

[papi] Add UserService/UpdateUser to proto #19163

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4846ad6
[papi] Add `UserService/UpdateUser` to proto
AlexTugarev Nov 29, 2023
1a0632b
extract SetWorkspaceAutoStartOptions
AlexTugarev Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions components/public-api/gitpod/v1/user.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,60 @@ option go_package = "github.com/gitpod-io/gitpod/components/public-api/go/v1";
service UserService {
// GetAuthenticatedUser allows to retrieve the current user.
rpc GetAuthenticatedUser(GetAuthenticatedUserRequest) returns (GetAuthenticatedUserResponse) {}

// UpdateUser updates the properties of a user.
rpc UpdateUser(UpdateUserRequest) returns (UpdateUserResponse) {}
}

message GetAuthenticatedUserRequest {}
message GetAuthenticatedUserResponse {
User user = 1;
}

message UpdateUserRequest {
string user_id = 1;
optional string name = 2;
optional string accepted_privacy_policy_date = 3;
optional ProfileDetails profile = 4;
akosyakov marked this conversation as resolved.
Show resolved Hide resolved
message ProfileDetails {
optional string last_updated_details_nudge = 1;
optional string accepted_privacy_policy_date = 2;
optional string company_name = 3;
optional string email_address = 4;
optional string job_role = 5;
optional string job_role_other = 6;
repeated string exploration_reasons = 7;
repeated string signup_goals = 8;
optional string signup_goals_other = 10;
optional string onboarded_timestamp = 11;
optional string company_size = 12;
}
optional string email_address = 5;
optional EditorReference editor_settings = 6;
optional EmailNotificationSettings email_notification_settings = 7;
message EmailNotificationSettings {
optional bool allows_changelog_mail = 1;
optional bool allows_devx_mail = 2;
optional bool allows_onboarding_mail = 3;
}
repeated WorkspaceAutostartOption workspace_autostart_options = 8;
message WorkspaceAutostartOption {
string clone_url = 1;
Copy link
Member

Choose a reason for hiding this comment

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

all properties should be optional then

Copy link
Member Author

Choose a reason for hiding this comment

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

@akosyakov the workspace_autostart_options is a repeated field. The way you would update it is to provide a complete new array of values of type WorkspaceAutostartOption. Signaling optional here would suggest the system could update an individual entry by specifying a diff, but how should that be implemented?
The simpler solution is, if workspace_autostart_options is not provided (it's optional anyways), we're not touching this in persistent state. If it's an empty array, we reset the state. If there are entries, we store this as new state.

Copy link
Member

Choose a reason for hiding this comment

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

We discussed to move it out to own SetWorkspaceAutostartOptions which is not partial and support current dashboard.

Copy link
Member Author

Choose a reason for hiding this comment

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

resolved via 1a0632b

string organization_id = 2;
string workspace_class = 3;
EditorReference editor_settings = 4;
akosyakov marked this conversation as resolved.
Show resolved Hide resolved
string region = 5;
}
optional WorkspaceTimeoutSettings workspace_timeout_settings = 9;
message WorkspaceTimeoutSettings {
optional google.protobuf.Duration inactivity = 1;
optional bool disabled_disconnected = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

we have another workspace timeout in Refind workspace proto PR https://github.com/gitpod-io/gitpod/pull/19138/files#diff-393d8178f08b8814eea780cf779ac93bc55fd0b90588e907a825cbb51de9f33aR250

Should them be aligned?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's align them in implementation PR.
TBH, this is already confusing enough. Changed that already twice, and it's unclear how to map them. In the beginning we'll need to map them in order to maintain compatibility in FE shim, right?

Here I did the bare minimum for the conversion.
https://github.com/gitpod-io/gitpod/pull/19142/files#diff-8fdb520274403fe903bd0ad9c9094f3951b4940b9cf838aa5c36b6548f9aafbdR964-R967

Happy to align, if there is an example for the conversion available.

}
}
message UpdateUserResponse {
User user = 1;
}

message User {
// id is a UUID of the user
string id = 1;
Expand Down
Loading
Loading