Follow-up from ROP part 3 - split up classes into smaller methods
MR: Pending
## Description
The following discussion from !125358 should be addressed:
- [ ] @a_akgun started a [discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125358#note_1469663560): (+3 comments)
> optional:
>
> @cwoolley-gitlab we could divide the `create` method into more methods, for example we could move some of the stuff here into a private method, like `workspace_params` etc.
>
> Similar concern for all other `ee/lib/remote_development/workspaces/create/devfile_fetcher.rb`
## Potential Concerns
Follow-up comment from @cwoolley-gitlab :
---
It would end up looking like the following, which makes for a smaller entry point method, but makes us pass around a lot of arguments for the variables we destructure out of the `value` arg. I just returned them from `destructure_value` as an array, which is order-dependent - we could make them a hash if we wanted, but then we'd have to destructure them out of that hash again to use them in `build_workspace`, which seems excessive.
```ruby
module RemoteDevelopment
module Workspaces
module Create
# noinspection RubyResolve - Rubymine isn't detecting ActiveRecord db field properties of workspace
class Creator
include States
include Messages
# @param [Hash] value
# @return [Result]
def self.create(value)
agent, devfile_yaml, params, processed_devfile, project, user, workspace_root = workspace_params(value)
workspace = build_workspace(
agent: agent,
devfile_yaml: devfile_yaml,
params: params,
processed_devfile: processed_devfile,
project: project,
user: user,
workspace_root: workspace_root
)
if workspace.save
Result.ok(
WorkspaceCreateSuccessful.new(
value.merge({
workspace: workspace
})
)
)
else
Result.err(WorkspaceCreateFailed.new({ errors: workspace.errors }))
end
end
# @param [Clusters::Agent] agent
# @param [String] devfile_yaml
# @param [Hash] params
# @param [Hash] processed_devfile
# @param [Project] project
# @param [User] user
# @param [String] workspace_root
# @return [Workspace]
def self.build_workspace(agent:, devfile_yaml:, params:, processed_devfile:, project:, user:, workspace_root:)
processed_devfile_yaml = YAML.dump(processed_devfile.deep_stringify_keys)
workspace = project.workspaces.build(params)
workspace.devfile = devfile_yaml
workspace.processed_devfile = processed_devfile_yaml
workspace.actual_state = CREATION_REQUESTED
random_string = SecureRandom.alphanumeric(6).downcase
workspace.namespace = "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}"
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409774
# We can come maybe come up with a better/cooler way to get a unique name, for now this works
workspace.name = "workspace-#{agent.id}-#{user.id}-#{random_string}"
project_dir = "#{workspace_root}/#{project.path}"
workspace_host = "60001-#{workspace.name}.#{workspace.dns_zone}"
workspace.url = URI::HTTPS.build({
host: workspace_host,
query: {
folder: project_dir
}.to_query
}).to_s
workspace
end
# @param [Hash] value
# @return [Array]
def self.workspace_params(value)
value => {
devfile_yaml: String => devfile_yaml,
processed_devfile: Hash => processed_devfile,
volume_mounts: Hash => volume_mounts,
current_user: User => user,
params: Hash => params,
}
volume_mounts => { data_volume: Hash => data_volume }
data_volume => {
path: String => workspace_root,
}
params => {
project: Project => project,
agent: Clusters::Agent => agent,
}
[agent, devfile_yaml, params, processed_devfile, project, user, workspace_root]
end
end
end
end
end
```
Is this what you had in mind?
Ordinarily I'm all for extracting things to small, well-named, cohesive methods, but in this case, I'm not sure if this makes the code any more understandable.
## Agreed Solution
Based on the [thread below](https://gitlab.com/gitlab-org/gitlab/-/issues/418754#note_1472948216), we will create private methods where it makes sense to extract logic, but **_NOT_** for the value-destructuring-to-local-variables logic.
## Acceptance Criteria
- [ ] Existing ROP classes are split into smaller methods
## Impact Assessment
Makes it easier to understand the code.
<!-- Replace with other type, e.g. bug or maintenance, if appropriate -->
<!-- Replace with other subtype if appropriate -->
<!-- By default, all issues start in the unprioritized status. See https://about.gitlab.com/handbook/engineering/development/dev/create/ide/#-remote-development-planning-process -->
<!-- For simplicity and to avoid triage bot warnings about missing workflow labels, we will default to issues starting at the refinement phase -->
issue