From 7b2c6cf018c4985d5b4dff69ea135b854567d04c Mon Sep 17 00:00:00 2001 From: Marcel van Remmerden Date: Wed, 28 Aug 2024 16:11:08 +0000 Subject: [PATCH 1/5] Improve layout of workspace items - Redesign the User Workspaces UI to simplify how we communicate a Workspace's state and other important information. - Create a calculateDisplayFunction function to encapsulate the Workspace's display state logic. As a result, the implementation of Vue components that depend on the "display state" is significantly simpler. The workspace's displayState is calculated based on the Workspace's actualState and desiredState properties. It creates an Storybook story that visualizes a Workspace's displayState for every combination of a Workspace's actualState and desiredState values. Changelog: changed EE: true --- .../components/open_workspace_button.vue | 57 +++++ .../common/components/workspace_actions.vue | 135 ++++------- .../components/workspace_state_indicator.vue | 8 +- .../workspaces_table.stories.js | 49 ++++ .../workspaces_list/workspaces_table.vue | 204 +++++----------- .../services/calculate_display_state.js | 83 +++++++ .../components/workspace_dropdown_item.vue | 18 +- .../workspaces/user/pages/list.stories.js | 28 --- ee/app/models/remote_development/workspace.rb | 4 +- .../workspaces_dropdown_group_spec.rb | 4 +- .../remote_development/workspaces_spec.rb | 223 ++++++++++-------- .../components/open_workspace_button_spec.js | 74 ++++++ .../components/workspace_actions_spec.js | 159 +++---------- .../workspace_state_indicator_spec.js | 36 ++- .../workspaces_list/workspaces_table_spec.js | 95 +++----- .../services/calculate_display_state_spec.js | 57 +++++ .../workspace_dropdown_item_spec.js | 11 +- .../remote_development/workspace_spec.rb | 2 +- locale/gitlab.pot | 15 +- qa/qa/ee/page/workspace/action.rb | 3 +- 20 files changed, 661 insertions(+), 604 deletions(-) create mode 100644 ee/app/assets/javascripts/workspaces/common/components/open_workspace_button.vue create mode 100644 ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.stories.js create mode 100644 ee/app/assets/javascripts/workspaces/common/services/calculate_display_state.js delete mode 100644 ee/app/assets/javascripts/workspaces/user/pages/list.stories.js create mode 100644 ee/spec/frontend/workspaces/common/components/open_workspace_button_spec.js create mode 100644 ee/spec/frontend/workspaces/common/services/calculate_display_state_spec.js diff --git a/ee/app/assets/javascripts/workspaces/common/components/open_workspace_button.vue b/ee/app/assets/javascripts/workspaces/common/components/open_workspace_button.vue new file mode 100644 index 00000000000000..bba1be8527a181 --- /dev/null +++ b/ee/app/assets/javascripts/workspaces/common/components/open_workspace_button.vue @@ -0,0 +1,57 @@ + + diff --git a/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue b/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue index 57b80d8a48211c..ab191899a192a7 100644 --- a/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue +++ b/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue @@ -1,143 +1,94 @@ diff --git a/ee/app/assets/javascripts/workspaces/common/components/workspace_state_indicator.vue b/ee/app/assets/javascripts/workspaces/common/components/workspace_state_indicator.vue index 73c81d0d8389c9..502fd28baf1fb1 100644 --- a/ee/app/assets/javascripts/workspaces/common/components/workspace_state_indicator.vue +++ b/ee/app/assets/javascripts/workspaces/common/components/workspace_state_indicator.vue @@ -46,7 +46,7 @@ export default { GlTooltip: GlTooltipDirective, }, props: { - workspaceState: { + workspaceDisplayState: { type: String, required: true, validator: (value) => Object.values(WORKSPACE_STATES).includes(value), @@ -54,13 +54,13 @@ export default { }, computed: { iconName() { - return stateLabel.includes(this.workspaceState) ? 'status' : ''; + return stateLabel.includes(this.workspaceDisplayState) ? 'status' : ''; }, iconLabel() { - return i18n.labels[this.workspaceState]; + return i18n.labels[this.workspaceDisplayState]; }, variant() { - return STATE_TO_VARIANT[this.workspaceState]; + return STATE_TO_VARIANT[this.workspaceDisplayState]; }, }, }; diff --git a/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.stories.js b/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.stories.js new file mode 100644 index 00000000000000..871dd9c9366d8d --- /dev/null +++ b/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.stories.js @@ -0,0 +1,49 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import { withGitLabAPIAccess } from 'storybook_addons/gitlab_api_access'; +import { WORKSPACE_STATES, WORKSPACE_DESIRED_STATES } from '../../constants'; +import WorkspacesTable from './workspaces_table.vue'; + +Vue.use(VueApollo); + +const MOCK_WORKSPACES = Object.values(WORKSPACE_STATES).flatMap((actualState, i) => + Object.values(WORKSPACE_DESIRED_STATES).map((desiredState, j) => { + const randomId = `${i}-${j}`; + + return { + id: `gid://gitlab/RemoteDevelopment::Workspace/${randomId}`, + projectName: 'GitLab.org / GitLab Development Kit', + actualState, + desiredState, + devfileRef: 'main', + devfilePath: '.devfile.yaml', + url: 'https://60001-workspace-73241-3688647-4a3yqq.workspaces.gitlab.dev?folder=%2Fprojects%2Fgitlab-development-kit', + name: `workspace-73241-3688647 ${actualState} - ${desiredState}`, + createdAt: 1723834797000, + }; + }), +); + +const Template = (_, { argTypes, createVueApollo }) => { + return { + components: { WorkspacesTable }, + apolloProvider: createVueApollo(), + provide: { + emptyStateSvgPath: '', + }, + props: Object.keys(argTypes), + template: '', + }; +}; + +export default { + component: WorkspacesTable, + decorators: [withGitLabAPIAccess], + title: 'ee/workspaces/workspaces_table', +}; + +export const Default = Template.bind({}); + +Default.args = { + workspaces: MOCK_WORKSPACES, +}; diff --git a/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.vue b/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.vue index 29c2ce75a43902..a4ead1ee31b78c 100644 --- a/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.vue +++ b/ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.vue @@ -1,80 +1,27 @@ diff --git a/ee/app/assets/javascripts/workspaces/common/services/calculate_display_state.js b/ee/app/assets/javascripts/workspaces/common/services/calculate_display_state.js new file mode 100644 index 00000000000000..94494f6b446a9f --- /dev/null +++ b/ee/app/assets/javascripts/workspaces/common/services/calculate_display_state.js @@ -0,0 +1,83 @@ +import { WORKSPACE_DESIRED_STATES, WORKSPACE_STATES } from '../constants'; + +/** + * Calculates a Workspace's displayState based on the Workspace's actualState and + * desiredState. A "displayState" is a UI-specific state used by several components + * in the User's Workspaces application to determine how to communicate a Workspace's state + * and which actions are available in the UI. + * + * IMPLEMENTATION NOTE: The order of the rules implemented in this function matters. + * + * @param {String} workspaceActualState + * @param {String} workspaceDesiredState + * @returns {string} + */ +export const calculateDisplayState = (workspaceActualState, workspaceDesiredState) => { + /** + * 1st rule: The workspace is terminated. The workspace can't transition to other states + * from this actual state therefore is final. + */ + if (workspaceActualState === WORKSPACE_STATES.terminated) { + return WORKSPACE_STATES.terminated; + } + + /** + * 2nd rule: The user wants to terminate the workspace. The user can't cancel the operation + * and the workspace can't transition to other states after being terminated per the 1st rule. + */ + if (workspaceDesiredState === WORKSPACE_DESIRED_STATES.terminated) { + return WORKSPACE_STATES.terminating; + } + + /** + * 3rd rule: Actual state takes precedence over desired state when: + * - The workspace's actual state and the workspaces's desired state are the same. + * - The workspace's actual state is unknown, failed, error, or terminating. + * */ + if ( + workspaceActualState === workspaceDesiredState || + [ + WORKSPACE_STATES.unknown, + WORKSPACE_STATES.failed, + WORKSPACE_STATES.error, + WORKSPACE_STATES.terminating, + ].includes(workspaceActualState) + ) { + return workspaceActualState; + } + + /* + * 4th rule: If the workspace's desired state is Stopped, we display that the Workspace is stopping. + */ + if ([WORKSPACE_DESIRED_STATES.stopped].includes(workspaceDesiredState)) { + return WORKSPACE_STATES.stopping; + } + + /** + * 5th rule: If the workspace's desired state is RestartRequested, we display that the Workspace is stopping + * unless the workspace is already stopped. The backend stops a workspace when the desired state is RestartRequested + * and, after the workspace is stopped, the backend will set desiredState to running + * https://handbook.gitlab.com/handbook/engineering/architecture/design-documents/workspaces/#possible-desired_state-values + */ + if (workspaceDesiredState === WORKSPACE_DESIRED_STATES.restartRequested) { + return workspaceActualState === WORKSPACE_STATES.stopped + ? WORKSPACE_STATES.starting + : WORKSPACE_STATES.stopping; + } + + /** + * 6th rule: If the workspace's desired state is Running, display that the workspace is starting + * unless the workspace actual space is CreationRequested because we want to display a "Creating" label. + */ + if ( + workspaceDesiredState === WORKSPACE_DESIRED_STATES.running && + workspaceActualState !== WORKSPACE_STATES.creationRequested + ) { + return WORKSPACE_STATES.starting; + } + + /** + * If the rules above are not satisfied, it's safe to return the actual state. + */ + return workspaceActualState; +}; diff --git a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue index 0f49425d66d7ee..c95e254b287143 100644 --- a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue +++ b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue @@ -4,6 +4,7 @@ import Tracking from '~/tracking'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import WorkspaceStateIndicator from '../../common/components/workspace_state_indicator.vue'; import WorkspaceActions from '../../common/components/workspace_actions.vue'; +import { calculateDisplayState } from '../../common/services/calculate_display_state'; export default { components: { @@ -26,6 +27,9 @@ export default { text: this.workspace.name, }; }, + displayState() { + return calculateDisplayState(this.workspace.actualState, this.workspace.desiredState); + }, }, methods: { trackOpenWorkspace() { @@ -39,16 +43,16 @@ export default { diff --git a/ee/app/assets/javascripts/workspaces/user/pages/list.stories.js b/ee/app/assets/javascripts/workspaces/user/pages/list.stories.js deleted file mode 100644 index c6aeb9f48df9f9..00000000000000 --- a/ee/app/assets/javascripts/workspaces/user/pages/list.stories.js +++ /dev/null @@ -1,28 +0,0 @@ -import Vue from 'vue'; -import VueApollo from 'vue-apollo'; -import { withGitLabAPIAccess } from 'storybook_addons/gitlab_api_access'; -import WorkspacesList from './list.vue'; - -Vue.use(VueApollo); - -const Template = (_, { argTypes, createVueApollo }) => { - return { - components: { WorkspacesList }, - apolloProvider: createVueApollo(), - provide: { - emptyStateSvgPath: '', - }, - props: Object.keys(argTypes), - template: '', - }; -}; - -export default { - component: WorkspacesList, - title: 'ee/workspaces/workspaces_list', - decorators: [withGitLabAPIAccess], -}; - -export const Default = Template.bind({}); - -Default.args = {}; diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 30136dd3a18f2e..3fe7d762d0789d 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -144,7 +144,9 @@ def exceeds_workspaces_quota? end def url - URI::HTTPS.build(host: "#{url_prefix}.#{workspaces_agent_config.dns_zone}", query: url_query_string).to_s + URI::HTTPS.build(host: "#{url_prefix}.#{workspaces_agent_config.dns_zone}", + path: '/', + query: url_query_string).to_s end def devfile_web_url diff --git a/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb b/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb index bfb535bed442af..1aaa8e8a2b4ce2 100644 --- a/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb +++ b/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb @@ -94,6 +94,8 @@ # Asserts the workspace state is correctly displayed expect_workspace_state_indicator(workspace.actual_state) + click_button('Actions') + # Asserts that all workspaces actions are visible expect(page).to have_button('Restart') expect(page).to have_button('Stop') @@ -102,7 +104,7 @@ click_button('Stop') # Ensures that the user can change a workspace state - expect(page).to have_button('Stopping', disabled: true) + expect(page).to have_content('Stopping') end # @param [String] state diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index 740fee27372fbe..676f8522fbc903 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -79,116 +79,129 @@ # We look for the project GID because that's all we know about the workspace at this point. For the new UI, # we will have to either expose this as a field on the new workspaces UI, or else come up # with some more clever finder to assert on the workspace showing up in the list after a refresh. - page.find('td', text: project.name_with_namespace) + page.find('span[data-testid="workspaces-project-name"]', text: project.name_with_namespace) # GET NAME AND NAMESPACE OF NEW WORKSPACE workspaces = RemoteDevelopment::Workspace.all.to_a expect(workspaces.length).to eq(1) workspace = workspaces[0] - - # ASSERT ON NEW WORKSPACE IN LIST - page.find('td', text: workspace.name) - - # ASSERT WORKSPACE STATE BEFORE POLLING NEW STATES - expect_workspace_state_indicator('Creating') - - # ASSERT TERMINATE BUTTON IS AVAILABLE - expect(page).to have_button('Terminate') - - additional_args_for_expected_config_to_apply = - build_additional_args_for_expected_config_to_apply( - network_policy_enabled: true, - dns_zone: agent.unversioned_latest_workspaces_agent_config.dns_zone, - namespace_path: group.path, - project_name: project.path - ) - - # SIMULATE FIRST POLL FROM AGENTK TO PICK UP NEW WORKSPACE - simulate_first_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # SIMULATE SECOND POLL FROM AGENTK TO UPDATE WORKSPACE TO RUNNING STATE - simulate_second_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS RUNNING STATE IN UI AND UPDATES URL - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::RUNNING) - expect(page).to have_selector('a', text: workspace.url) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR RUNNING STATE - expect(page).to have_button('Restart') - expect(page).to have_button('Stop') - expect(page).to have_button('Terminate') - - click_button 'Stop' - - # SIMULATE THIRD POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPING STATE - simulate_third_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS STOPPING STATE IN UI - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPING) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPING STATE - # TODO: What other buttons are there? - expect(page).to have_button('Terminate') - - # SIMULATE FOURTH POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPED STATE - simulate_fourth_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS STOPPED STATE IN UI - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPED) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPED STATE - expect(page).to have_button('Start') - expect(page).to have_button('Terminate') - - # SIMULATE FIFTH POLL FROM AGENTK FOR PARTIAL RECONCILE TO SHOW NO RAILS_INFOS ARE SENT - simulate_fifth_poll do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) + within_workspace workspace do + # ASSERT ON NEW WORKSPACE IN LIST + expect(page).to have_content(workspace.name) + + # ASSERT WORKSPACE STATE BEFORE POLLING NEW STATES + expect_workspace_state_indicator('Creating') + + # ASSERT TERMINATE BUTTON IS AVAILABLE + click_button 'Actions' + expect(page).to have_button('Terminate') + + # CLOSE THE ACTIONS DROPDOWN + click_button 'Actions' + + additional_args_for_expected_config_to_apply = + build_additional_args_for_expected_config_to_apply( + network_policy_enabled: true, + dns_zone: agent.unversioned_latest_workspaces_agent_config.dns_zone, + namespace_path: group.path, + project_name: project.path + ) + + # SIMULATE FIRST POLL FROM AGENTK TO PICK UP NEW WORKSPACE + simulate_first_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # SIMULATE SECOND POLL FROM AGENTK TO UPDATE WORKSPACE TO RUNNING STATE + simulate_second_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS RUNNING STATE IN UI AND UPDATES URL + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::RUNNING) + expect(find_open_workspace_button).to have_text('Open workspace') + expect(find_open_workspace_button[:href]).to eq(workspace.url) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR RUNNING STATE + click_button 'Actions' + expect(page).to have_button('Restart') + expect(page).to have_button('Stop') + expect(page).to have_button('Terminate') + + click_button 'Stop' + + # SIMULATE THIRD POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPING STATE + simulate_third_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS STOPPING STATE IN UI + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPING) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPING STATE + click_button 'Actions' + expect(page).to have_button('Terminate') + + # SIMULATE FOURTH POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPED STATE + simulate_fourth_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS STOPPED STATE IN UI + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPED) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPED STATE + expect(page).to have_button('Start') + expect(page).to have_button('Terminate') + + # SIMULATE FIFTH POLL FROM AGENTK FOR PARTIAL RECONCILE TO SHOW NO RAILS_INFOS ARE SENT + simulate_fifth_poll do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # SIMULATE SIXTH POLL FROM AGENTK FOR FULL RECONCILE TO SHOW ALL WORKSPACES ARE SENT IN RAILS_INFOS + simulate_sixth_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end end + end - # SIMULATE SIXTH POLL FROM AGENTK FOR FULL RECONCILE TO SHOW ALL WORKSPACES ARE SENT IN RAILS_INFOS - simulate_sixth_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) + def within_workspace(workspace) + page.within("div[data-testid=#{workspace.name}]") do + yield end end @@ -222,6 +235,10 @@ def simulate_agentk_reconcile_post(agent_token:, workspace_agent_infos:, update_ end end + def find_open_workspace_button + page.first('[data-testid="workspace-open-button"]', minimum: 0) + end + context 'when creating a workspace' do it_behaves_like 'creates a workspace' end diff --git a/ee/spec/frontend/workspaces/common/components/open_workspace_button_spec.js b/ee/spec/frontend/workspaces/common/components/open_workspace_button_spec.js new file mode 100644 index 00000000000000..66b5d8a1a442ab --- /dev/null +++ b/ee/spec/frontend/workspaces/common/components/open_workspace_button_spec.js @@ -0,0 +1,74 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlButton } from '@gitlab/ui'; +import OpenWorkspaceButton from 'ee/workspaces/common/components/open_workspace_button.vue'; +import { WORKSPACE_STATES } from 'ee/workspaces/common/constants'; + +describe('OpenWorkspaceButton', () => { + let wrapper; + + const createWrapper = ({ workspaceDisplayState, workspaceUrl }) => { + wrapper = shallowMount(OpenWorkspaceButton, { + propsData: { + workspaceDisplayState, + workspaceUrl, + }, + }); + }; + + describe(`when workspace display state is ${WORKSPACE_STATES.running}`, () => { + describe('when workspace has URL', () => { + it('displays "Open Workspace" button', () => { + createWrapper({ + workspaceDisplayState: WORKSPACE_STATES.running, + workspaceUrl: 'http://example.com/', + }); + + expect(wrapper.findComponent(GlButton).text()).toContain('Open workspace'); + }); + }); + + describe('when workspace does not have URL', () => { + it('does not display "Open Workspace" button', () => { + createWrapper({ + workspaceDisplayState: WORKSPACE_STATES.running, + workspaceUrl: '', + }); + + expect(wrapper.findComponent(GlButton).exists()).toBe(false); + }); + }); + }); + + describe.each([WORKSPACE_STATES.creationRequested, WORKSPACE_STATES.starting])( + `when workspace display state is %s`, + (workspaceDisplayState) => { + it('displays starting workspace loading button', () => { + createWrapper({ + workspaceDisplayState, + workspaceUrl: '', + }); + + expect(wrapper.findComponent(GlButton).text()).toContain('Starting workspace'); + }); + }, + ); + + describe.each([ + WORKSPACE_STATES.stopping, + WORKSPACE_STATES.stopped, + WORKSPACE_STATES.failed, + WORKSPACE_STATES.error, + WORKSPACE_STATES.unknown, + WORKSPACE_STATES.terminated, + WORKSPACE_STATES.terminating, + ])(`when workspace display state is %s`, (workspaceDisplayState) => { + it('displays no button', () => { + createWrapper({ + workspaceDisplayState, + workspaceUrl: 'http://example.com/', + }); + + expect(wrapper.findComponent(GlButton).exists()).toBe(false); + }); + }); +}); diff --git a/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js b/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js index 870fb0c4da3533..3de867d9294fa1 100644 --- a/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js +++ b/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js @@ -1,153 +1,72 @@ -import { shallowMount } from '@vue/test-utils'; -import { GlButton, GlTooltip } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import { GlDisclosureDropdownItem } from '@gitlab/ui'; import WorkspaceActions from 'ee/workspaces/common/components/workspace_actions.vue'; -import { - WORKSPACE_STATES as ACTUAL, - WORKSPACE_DESIRED_STATES as DESIRED, -} from 'ee/workspaces/common/constants'; +import { WORKSPACE_STATES as ACTUAL } from 'ee/workspaces/common/constants'; describe('ee/workspaces/components/common/workspace_actions', () => { let wrapper; const createWrapper = (props = {}) => { - wrapper = shallowMount(WorkspaceActions, { + wrapper = mount(WorkspaceActions, { propsData: { ...props, }, - stubs: { - GlTooltip, - }, }); }; - const findButtons = () => wrapper.findAllComponents(GlButton); - const findButtonWithLabel = (label) => - findButtons().wrappers.find((x) => x.attributes('aria-label') === label); - const findButtonsAsData = () => - findButtons().wrappers.map((button) => ({ - ariaLabel: button.attributes('aria-label'), - icon: button.props('icon'), - disabled: button.props('disabled'), - loading: button.props('loading'), - })); - const findTooltipsAsData = () => - wrapper.findAllComponents(GlTooltip).wrappers.map((tooltip) => ({ - text: tooltip.text(), - target: tooltip.props().target, + const findDropdownItems = () => wrapper.findAllComponents(GlDisclosureDropdownItem); + const findDropdownItemWithText = (text) => + findDropdownItems().wrappers.find((x) => x.text() === text); + const findDropdownItemsAsData = () => + findDropdownItems().wrappers.map((button) => ({ + text: button.text(), })); - const createButtonData = (tooltip, icon, loading = false) => ({ - icon, - ariaLabel: tooltip, - disabled: loading, - loading, + const createButtonData = (text) => ({ + text, }); - const RESTART_BUTTON = createButtonData('Restart', 'retry'); - const RESTARTING_BUTTON = createButtonData('Restarting', 'retry', true); - const START_BUTTON = createButtonData('Start', 'play'); - const STARTING_BUTTON = createButtonData('Starting', 'play', true); - const STOP_BUTTON = createButtonData('Stop', 'stop'); - const STOPPING_BUTTON = createButtonData('Stopping', 'stop', true); - const TERMINATE_BUTTON = createButtonData('Terminate', 'remove'); - const TERMINATING_BUTTON = createButtonData('Terminating', 'remove', true); + const RESTART_BUTTON = createButtonData('Restart'); + const START_BUTTON = createButtonData('Start'); + const STOP_BUTTON = createButtonData('Stop'); + const TERMINATE_BUTTON = createButtonData('Terminate'); it.each` - actualState | desiredState | buttonsData - ${ACTUAL.creationRequested} | ${DESIRED.running} | ${[STARTING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.creationRequested} | ${DESIRED.stopped} | ${[START_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.creationRequested} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.creationRequested} | ${DESIRED.restartRequested} | ${[TERMINATE_BUTTON]} - ${ACTUAL.starting} | ${DESIRED.running} | ${[STARTING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.starting} | ${DESIRED.stopped} | ${[START_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.starting} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.starting} | ${DESIRED.restartRequested} | ${[TERMINATE_BUTTON]} - ${ACTUAL.running} | ${DESIRED.running} | ${[RESTART_BUTTON, STOP_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.running} | ${DESIRED.stopped} | ${[STOPPING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.running} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.running} | ${DESIRED.restartRequested} | ${[RESTARTING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.stopping} | ${DESIRED.running} | ${[STOP_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.stopping} | ${DESIRED.stopped} | ${[STOPPING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.stopping} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.stopping} | ${DESIRED.restartRequested} | ${[TERMINATE_BUTTON]} - ${ACTUAL.stopped} | ${DESIRED.running} | ${[STARTING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.stopped} | ${DESIRED.stopped} | ${[START_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.stopped} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.terminated} | ${DESIRED.running} | ${[]} - ${ACTUAL.terminated} | ${DESIRED.stopped} | ${[]} - ${ACTUAL.terminated} | ${DESIRED.terminated} | ${[]} - ${ACTUAL.terminated} | ${DESIRED.restartRequested} | ${[]} - ${ACTUAL.failed} | ${DESIRED.running} | ${[RESTART_BUTTON, STOP_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.failed} | ${DESIRED.stopped} | ${[STOPPING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.failed} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.failed} | ${DESIRED.restartRequested} | ${[RESTARTING_BUTTON, TERMINATE_BUTTON]} - ${ACTUAL.error} | ${DESIRED.running} | ${[TERMINATE_BUTTON]} - ${ACTUAL.error} | ${DESIRED.stopped} | ${[TERMINATE_BUTTON]} - ${ACTUAL.error} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.error} | ${DESIRED.restartRequested} | ${[TERMINATE_BUTTON]} - ${ACTUAL.unknown} | ${DESIRED.running} | ${[]} - ${ACTUAL.unknown} | ${DESIRED.stopped} | ${[]} - ${ACTUAL.unknown} | ${DESIRED.terminated} | ${[]} - ${ACTUAL.unknown} | ${DESIRED.restartRequested} | ${[]} - ${ACTUAL.terminating} | ${DESIRED.running} | ${[TERMINATE_BUTTON]} - ${ACTUAL.terminating} | ${DESIRED.stopped} | ${[TERMINATE_BUTTON]} - ${ACTUAL.terminating} | ${DESIRED.terminated} | ${[TERMINATING_BUTTON]} - ${ACTUAL.terminating} | ${DESIRED.restartRequested} | ${[TERMINATE_BUTTON]} + workspaceDisplayState | buttonsData + ${ACTUAL.creationRequested} | ${[TERMINATE_BUTTON]} + ${ACTUAL.starting} | ${[TERMINATE_BUTTON]} + ${ACTUAL.running} | ${[RESTART_BUTTON, STOP_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.stopping} | ${[TERMINATE_BUTTON]} + ${ACTUAL.stopped} | ${[START_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.terminated} | ${[]} + ${ACTUAL.failed} | ${[RESTART_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.error} | ${[RESTART_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.unknown} | ${[RESTART_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.terminating} | ${[]} `( - 'renders buttons - with actualState=$actualState and desiredState=$desiredState', - ({ actualState, desiredState, buttonsData }) => { - createWrapper({ actualState, desiredState }); + 'renders buttons - with workspaceDisplayState=$workspaceDisplayState', + ({ workspaceDisplayState, buttonsData }) => { + createWrapper({ workspaceDisplayState }); - expect(findButtonsAsData()).toEqual(buttonsData); - expect(findTooltipsAsData()).toEqual( - buttonsData.map((buttonData) => ({ - text: buttonData.ariaLabel, - target: expect.stringMatching(/action-wrapper-\w+\d{1}/), - })), - ); + expect(findDropdownItemsAsData()).toEqual(buttonsData); }, ); it.each` - actualState | desiredState | buttonLabel | actionDesiredState - ${ACTUAL.creationRequested} | ${DESIRED.running} | ${'Terminate'} | ${'Terminated'} - ${ACTUAL.stopped} | ${DESIRED.stopped} | ${'Start'} | ${'Running'} - ${ACTUAL.running} | ${DESIRED.running} | ${'Stop'} | ${'Stopped'} + workspaceDisplayState | text | actionDesiredState + ${ACTUAL.creationRequested} | ${'Terminate'} | ${'Terminated'} + ${ACTUAL.stopped} | ${'Start'} | ${'Running'} + ${ACTUAL.running} | ${'Stop'} | ${'Stopped'} `( - 'when clicking "$buttonLabel", emits "click" with "$actionDesiredState"', - ({ actualState, desiredState, buttonLabel, actionDesiredState }) => { - const mockEvent = { stopPropagation: jest.fn(), preventDefault: jest.fn() }; - - createWrapper({ actualState, desiredState }); + 'when clicking "$text", emits "click" with "$actionDesiredState"', + async ({ workspaceDisplayState, text, actionDesiredState }) => { + createWrapper({ workspaceDisplayState }); expect(wrapper.emitted('click')).toBeUndefined(); - const button = findButtonWithLabel(buttonLabel); - - button.vm.$emit('click', mockEvent); + await findDropdownItemWithText(text).find('button').trigger('click'); expect(wrapper.emitted('click')).toEqual([[actionDesiredState]]); - expect(mockEvent.stopPropagation).toHaveBeenCalled(); - expect(mockEvent.preventDefault).toHaveBeenCalled(); }, ); - - describe('when compact mode', () => { - beforeEach(() => { - createWrapper({ - actualState: ACTUAL.creationRequested, - desiredState: DESIRED.running, - compact: true, - }); - }); - - it('sets buttons as small and category tertiary', () => { - findButtons().wrappers.forEach((button) => { - expect(button.props()).toMatchObject({ - category: 'tertiary', - size: 'small', - }); - }); - }); - }); }); diff --git a/ee/spec/frontend/workspaces/common/components/workspace_state_indicator_spec.js b/ee/spec/frontend/workspaces/common/components/workspace_state_indicator_spec.js index 18ae85aa2a5609..20b2af721eb0e2 100644 --- a/ee/spec/frontend/workspaces/common/components/workspace_state_indicator_spec.js +++ b/ee/spec/frontend/workspaces/common/components/workspace_state_indicator_spec.js @@ -1,37 +1,35 @@ import { shallowMount } from '@vue/test-utils'; import { GlBadge } from '@gitlab/ui'; -import WorkspaceStateIndicator, { - i18n, -} from 'ee/workspaces/common/components/workspace_state_indicator.vue'; +import WorkspaceStateIndicator from 'ee/workspaces/common/components/workspace_state_indicator.vue'; import { WORKSPACE_STATES } from 'ee/workspaces/common/constants'; describe('WorkspaceStateIndicator', () => { let wrapper; - const createWrapper = ({ workspaceState }) => { + const createWrapper = ({ workspaceDisplayState }) => { wrapper = shallowMount(WorkspaceStateIndicator, { propsData: { - workspaceState, + workspaceDisplayState, }, }); }; it.each` - workspaceState | iconName | label | variant - ${WORKSPACE_STATES.creationRequested} | ${'status'} | ${i18n.labels[WORKSPACE_STATES.creationRequested]} | ${'success'} - ${WORKSPACE_STATES.starting} | ${'status'} | ${i18n.labels[WORKSPACE_STATES.starting]} | ${'success'} - ${WORKSPACE_STATES.running} | ${''} | ${i18n.labels[WORKSPACE_STATES.running]} | ${'success'} - ${WORKSPACE_STATES.stopping} | ${'status'} | ${i18n.labels[WORKSPACE_STATES.stopping]} | ${'info'} - ${WORKSPACE_STATES.stopped} | ${''} | ${i18n.labels[WORKSPACE_STATES.stopped]} | ${'info'} - ${WORKSPACE_STATES.failed} | ${''} | ${i18n.labels[WORKSPACE_STATES.failed]} | ${'danger'} - ${WORKSPACE_STATES.error} | ${''} | ${i18n.labels[WORKSPACE_STATES.error]} | ${'danger'} - ${WORKSPACE_STATES.unknown} | ${''} | ${i18n.labels[WORKSPACE_STATES.unknown]} | ${'danger'} - ${WORKSPACE_STATES.terminating} | ${'status'} | ${i18n.labels[WORKSPACE_STATES.terminating]} | ${'muted'} - ${WORKSPACE_STATES.terminated} | ${''} | ${i18n.labels[WORKSPACE_STATES.terminated]} | ${'muted'} + workspaceDisplayState | iconName | label | variant + ${WORKSPACE_STATES.creationRequested} | ${'status'} | ${'Creating'} | ${'success'} + ${WORKSPACE_STATES.starting} | ${'status'} | ${'Starting'} | ${'success'} + ${WORKSPACE_STATES.running} | ${''} | ${'Running'} | ${'success'} + ${WORKSPACE_STATES.stopping} | ${'status'} | ${'Stopping'} | ${'info'} + ${WORKSPACE_STATES.stopped} | ${''} | ${'Stopped'} | ${'info'} + ${WORKSPACE_STATES.terminating} | ${'status'} | ${'Terminating'} | ${'muted'} + ${WORKSPACE_STATES.terminated} | ${''} | ${'Terminated'} | ${'muted'} + ${WORKSPACE_STATES.failed} | ${''} | ${'Failed'} | ${'danger'} + ${WORKSPACE_STATES.error} | ${''} | ${'Error'} | ${'danger'} + ${WORKSPACE_STATES.unknown} | ${''} | ${'Unknown state'} | ${'danger'} `( - 'displays $iconName with $tooltip and $cssClass when workspace state is $state', - ({ workspaceState, iconName, label, variant }) => { - createWrapper({ workspaceState }); + 'label=$label, icon=$iconName, variant=$variant when displayState=$workspaceDisplayState', + ({ workspaceDisplayState, iconName, label, variant }) => { + createWrapper({ workspaceDisplayState }); const badge = wrapper.findComponent(GlBadge); diff --git a/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js b/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js index 066760e58817a6..420e36b1749be8 100644 --- a/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js +++ b/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js @@ -1,15 +1,15 @@ import { mount } from '@vue/test-utils'; -import { cloneDeep } from 'lodash'; import VueApollo from 'vue-apollo'; import Vue from 'vue'; -import { GlLink, GlTableLite } from '@gitlab/ui'; import { useFakeDate } from 'helpers/fake_date'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import WorkspacesTable from 'ee/workspaces/common/components/workspaces_list/workspaces_table.vue'; import WorkspaceActions from 'ee/workspaces/common/components/workspace_actions.vue'; +import OpenWorkspaceButton from 'ee/workspaces/common/components/open_workspace_button.vue'; import WorkspaceStateIndicator from 'ee/workspaces/common/components/workspace_state_indicator.vue'; +import { calculateDisplayState } from 'ee/workspaces/common/services/calculate_display_state'; import { populateWorkspacesWithProjectDetails } from 'ee/workspaces/common/services/utils'; -import { WORKSPACE_STATES, WORKSPACE_DESIRED_STATES } from 'ee/workspaces/common/constants'; +import { WORKSPACE_DESIRED_STATES } from 'ee/workspaces/common/constants'; import { USER_WORKSPACES_LIST_QUERY_RESULT, GET_PROJECTS_DETAILS_QUERY_RESULT, @@ -21,34 +21,25 @@ Vue.use(VueApollo); const SVG_PATH = '/assets/illustrations/empty_states/empty_workspaces.svg'; -const findTable = (wrapper) => wrapper.findComponent(GlTableLite); -const findTableRows = (wrapper) => findTable(wrapper).findAll('tbody tr'); +const findTable = (wrapper) => wrapper.find('[data-testid="workspaces-list"]'); +const findTableRows = (wrapper) => wrapper.findAll('[data-testid="workspaces-list"] > li'); const findTableRowsAsData = (wrapper) => findTableRows(wrapper).wrappers.map((x) => { - const tds = x.findAll('td'); const rowData = { - workspaceState: tds.at(0).findComponent(WorkspaceStateIndicator).props('workspaceState'), - nameText: tds.at(1).text(), - createdAt: tds.at(2).findComponent(TimeAgoTooltip).props().time, - terminates: tds.at(3).text(), - actionsProps: tds.at(6).findComponent(WorkspaceActions).props(), + stateIndicatorProps: { + ...x.findComponent(WorkspaceStateIndicator).props(), + }, + openWorkspaceButtonProps: { + ...x.findComponent(OpenWorkspaceButton).props(), + }, + workspaceActionsProps: { + ...x.findComponent(WorkspaceActions).props(), + }, + nameText: x.find('[data-testid="workspace-name"]').text(), + createdAt: x.findComponent(TimeAgoTooltip).props().time, + terminates: x.find('[data-testid="workspace-termination"]').text(), }; - const devfileLinkTd = tds.at(4); - const devfileLink = devfileLinkTd.findComponent(GlLink); - if (devfileLink.exists()) { - rowData.devfileText = devfileLinkTd.text(); - rowData.devfileHref = devfileLink.attributes('href'); - rowData.devfileTooltipTitle = devfileLink.attributes('title'); - rowData.devfileTooltipAriaLabel = devfileLink.attributes('aria-label'); - } - - const previewLinkTd = tds.at(5); - if (previewLinkTd.findComponent(GlLink).exists()) { - rowData.previewText = previewLinkTd.text(); - rowData.previewHref = previewLinkTd.findComponent(GlLink).attributes('href'); - } - return rowData; }); const findWorkspaceActions = (tableRow) => tableRow.findComponent(WorkspaceActions); @@ -87,22 +78,6 @@ describe('workspaces/common/components/workspaces_list/workspaces_table.vue', () }); }; - const setupMockTerminatedWorkspace = (extraData = {}) => { - const customData = cloneDeep( - USER_WORKSPACES_LIST_QUERY_RESULT.data.currentUser.workspaces.nodes, - ); - const workspace = cloneDeep(customData[0]); - - customData.unshift({ - ...workspace, - name: 'workspace-1-1-idma03', - actualState: WORKSPACE_STATES.terminated, - ...extraData, - }); - - return customData; - }; - const findUpdateWorkspaceMutation = () => wrapper.findComponent(UpdateWorkspaceMutationStub); describe('default (with nodes)', () => { @@ -120,39 +95,25 @@ describe('workspaces/common/components/workspaces_list/workspaces_table.vue', () USER_WORKSPACES_LIST_QUERY_RESULT.data.currentUser.workspaces.nodes, GET_PROJECTS_DETAILS_QUERY_RESULT.data.projects.nodes, ).map((x) => { + const workspaceDisplayState = calculateDisplayState(x.actualState, x.desiredState); return { - nameText: `${x.projectName} ${x.name}`, - workspaceState: x.actualState, + nameText: x.name, createdAt: x.createdAt, terminates: x.name === 'workspace-1-1-idmi02' ? 'in 54 minutes' : 'in 2 days', - actionsProps: { - actualState: x.actualState, - desiredState: x.desiredState, - compact: false, + workspaceActionsProps: { + workspaceDisplayState, + }, + openWorkspaceButtonProps: { + workspaceDisplayState, + workspaceUrl: x.url, + }, + stateIndicatorProps: { + workspaceDisplayState, }, - devfileText: `${x.devfilePath} on ${x.devfileRef}`, - devfileHref: x.devfileWebUrl, - devfileTooltipTitle: x.devfileWebUrl, - ...(x.actualState === WORKSPACE_STATES.running - ? { - previewText: x.url, - previewHref: x.url, - } - : {}), }; }), ); }); - - describe('when the query returns terminated workspaces', () => { - beforeEach(() => { - createWrapper({ workspaces: setupMockTerminatedWorkspace() }); - }); - - it('sorts the list to display terminated workspaces at the end of the list', () => { - expect(findTableRowsAsData(wrapper).pop().workspaceState).toBe(WORKSPACE_STATES.terminated); - }); - }); }); describe.each` diff --git a/ee/spec/frontend/workspaces/common/services/calculate_display_state_spec.js b/ee/spec/frontend/workspaces/common/services/calculate_display_state_spec.js new file mode 100644 index 00000000000000..f36fc6c18fc10f --- /dev/null +++ b/ee/spec/frontend/workspaces/common/services/calculate_display_state_spec.js @@ -0,0 +1,57 @@ +import { calculateDisplayState } from 'ee/workspaces/common/services/calculate_display_state'; +import { WORKSPACE_STATES, WORKSPACE_DESIRED_STATES } from 'ee/workspaces/common/constants'; + +describe('workspaces/services/calculate_display_state', () => { + /** + * This test exercises all the possible combinations of actualState and desiredState even if not + * all combinations are possible. For example, the user can't try to start a workspace that errored. + */ + it.each` + workspaceActualState | workspaceDesiredState | result + ${WORKSPACE_STATES.creationRequested} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.creationRequested} + ${WORKSPACE_STATES.creationRequested} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.creationRequested} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.creationRequested} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.starting} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.starting} + ${WORKSPACE_STATES.starting} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.starting} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.starting} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.running} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.running} + ${WORKSPACE_STATES.running} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.running} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.running} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.stopping} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.starting} + ${WORKSPACE_STATES.stopping} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.stopping} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.stopping} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.stopping} + ${WORKSPACE_STATES.stopped} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.starting} + ${WORKSPACE_STATES.stopped} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.stopped} + ${WORKSPACE_STATES.stopped} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.stopped} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.starting} + ${WORKSPACE_STATES.terminating} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.terminating} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.terminating} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.terminating} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.terminated} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.terminated} + ${WORKSPACE_STATES.terminated} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.terminated} + ${WORKSPACE_STATES.terminated} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminated} + ${WORKSPACE_STATES.terminated} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.terminated} + ${WORKSPACE_STATES.failed} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.failed} + ${WORKSPACE_STATES.failed} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.failed} + ${WORKSPACE_STATES.failed} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.failed} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.failed} + ${WORKSPACE_STATES.error} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.error} + ${WORKSPACE_STATES.error} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.error} + ${WORKSPACE_STATES.error} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.error} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.error} + ${WORKSPACE_STATES.unknown} | ${WORKSPACE_DESIRED_STATES.running} | ${WORKSPACE_STATES.unknown} + ${WORKSPACE_STATES.unknown} | ${WORKSPACE_DESIRED_STATES.stopped} | ${WORKSPACE_STATES.unknown} + ${WORKSPACE_STATES.unknown} | ${WORKSPACE_DESIRED_STATES.terminated} | ${WORKSPACE_STATES.terminating} + ${WORKSPACE_STATES.unknown} | ${WORKSPACE_DESIRED_STATES.restartRequested} | ${WORKSPACE_STATES.unknown} + `( + 'label=$label, icon=$iconName, variant=$variant when actualState=$workspaceActualState and desiredState=$workspaceDesiredState', + ({ workspaceActualState, workspaceDesiredState, result }) => { + expect(calculateDisplayState(workspaceActualState, workspaceDesiredState)).toBe(result); + }, + ); +}); diff --git a/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js b/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js index 56e0431a5a1ec7..2179cdcce3849e 100644 --- a/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js +++ b/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js @@ -6,6 +6,7 @@ import WorkspaceStateIndicator from 'ee/workspaces/common/components/workspace_s import WorkspaceActions from 'ee/workspaces/common/components/workspace_actions.vue'; import { WORKSPACE_DESIRED_STATES } from 'ee/workspaces/dropdown_group/constants'; import WorkspaceDropdownItem from 'ee/workspaces/dropdown_group/components/workspace_dropdown_item.vue'; +import { calculateDisplayState } from 'ee/workspaces/common/services/calculate_display_state'; import { WORKSPACE } from '../../mock_data'; describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () => { @@ -27,12 +28,14 @@ describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () const findDropdownItem = () => wrapper.findComponent(GlDisclosureDropdownItem); describe('default', () => { + const displayState = calculateDisplayState(WORKSPACE.actualState, WORKSPACE.desiredState); + beforeEach(() => { createWrapper(); }); it('displays workspace state indicator', () => { - expect(findWorkspaceStateIndicator().props().workspaceState).toBe(WORKSPACE.actualState); + expect(findWorkspaceStateIndicator().props().workspaceDisplayState).toBe(displayState); }); it('displays the workspace name', () => { @@ -51,11 +54,7 @@ describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () }); it('displays workspace actions', () => { - expect(findWorkspaceActions().props()).toEqual({ - actualState: WORKSPACE.actualState, - desiredState: WORKSPACE.desiredState, - compact: true, - }); + expect(findWorkspaceActions().props().workspaceDisplayState).toEqual(displayState); }); }); diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 1e8d1c34ad3f42..b86704b95a39f2 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -91,7 +91,7 @@ describe '#url' do it 'returns calculated url' do - expect(workspace.url).to eq("https://60001-#{workspace.name}.#{agent_dns_zone}?folder=dir%2Ffile") + expect(workspace.url).to eq("https://60001-#{workspace.name}.#{agent_dns_zone}/?folder=dir%2Ffile") end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f93e201025011a..d2a5e665da5fb2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19511,9 +19511,6 @@ msgstr "" msgid "Development widget is not enabled for this work item type" msgstr "" -msgid "Devfile" -msgstr "" - msgid "Device name" msgstr "" @@ -63226,6 +63223,9 @@ msgstr "" msgid "Workspaces|No terminated workspaces" msgstr "" +msgid "Workspaces|Open workspace" +msgstr "" + msgid "Workspaces|Path to devfile" msgstr "" @@ -63235,9 +63235,6 @@ msgstr "" msgid "Workspaces|Restart" msgstr "" -msgid "Workspaces|Restarting" -msgstr "" - msgid "Workspaces|Running" msgstr "" @@ -63247,6 +63244,9 @@ msgstr "" msgid "Workspaces|Starting" msgstr "" +msgid "Workspaces|Starting workspace" +msgstr "" + msgid "Workspaces|Stop" msgstr "" @@ -63304,6 +63304,9 @@ msgstr "" msgid "Workspaces|Workspaces Settings" msgstr "" +msgid "Workspaces|You can open the workspace only once it is ready." +msgstr "" + msgid "Workspaces|You can't create a workspace for this project" msgstr "" diff --git a/qa/qa/ee/page/workspace/action.rb b/qa/qa/ee/page/workspace/action.rb index 42452fa32fcda1..54f940198aa35e 100644 --- a/qa/qa/ee/page/workspace/action.rb +++ b/qa/qa/ee/page/workspace/action.rb @@ -6,7 +6,8 @@ module Page module Workspace class Action < QA::Page::Base view 'ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue' do - element 'workspace-button', ':data-testid="`workspace-${action.key}-button`"' # rubocop:disable QA/ElementWithPattern -- Pattern to fetch workspace action dynamically + element 'workspace-actions-dropdown' + element 'workspace-button', ':data-testid="`workspace-${item.key}-button`"' # rubocop:disable QA/ElementWithPattern -- Pattern to fetch workspace action dynamically end view 'ee/app/assets/javascripts/workspaces/common/components/workspaces_list/workspaces_table.vue' do -- GitLab From e212f64a812c6ae1e928fda6c3f9a49279f28b0a Mon Sep 17 00:00:00 2001 From: Enrique Alcantara Date: Tue, 29 Oct 2024 11:34:58 +0100 Subject: [PATCH 2/5] UX and accessibility improvements - Fix nested interactive elements in the workspaces dropdown group by converting the Workspace name into a link that opens a workspace instead of using a workspace dropdown item - Restart action is not available in the Running state because it is redundant. --- .../common/components/workspace_actions.vue | 9 +- .../components/workspace_dropdown_item.vue | 62 +++++++++---- .../components/workspaces_dropdown_group.vue | 10 ++- .../workspaces_dropdown_group_spec.rb | 1 - .../remote_development/workspaces_spec.rb | 1 - .../components/workspace_actions_spec.js | 2 +- .../workspace_dropdown_item_spec.js | 87 ++++++++++++------- .../remote_development/integration_spec.rb | 1 + locale/gitlab.pot | 3 + 9 files changed, 115 insertions(+), 61 deletions(-) diff --git a/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue b/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue index ab191899a192a7..e319dfbcf374ce 100644 --- a/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue +++ b/ee/app/assets/javascripts/workspaces/common/components/workspace_actions.vue @@ -8,12 +8,9 @@ const ACTIONS = [ { key: 'restart', isVisible: (displayState) => - [ - WORKSPACE_STATES.running, - WORKSPACE_STATES.failed, - WORKSPACE_STATES.error, - WORKSPACE_STATES.unknown, - ].includes(displayState), + [WORKSPACE_STATES.failed, WORKSPACE_STATES.error, WORKSPACE_STATES.unknown].includes( + displayState, + ), desiredState: WORKSPACE_DESIRED_STATES.restartRequested, title: s__('Workspaces|Restart'), }, diff --git a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue index c95e254b287143..7c5253e9c88ea4 100644 --- a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue +++ b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspace_dropdown_item.vue @@ -1,18 +1,23 @@ diff --git a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspaces_dropdown_group.vue b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspaces_dropdown_group.vue index 2af44a2f76ff93..1eea534e6a2671 100644 --- a/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspaces_dropdown_group.vue +++ b/ee/app/assets/javascripts/workspaces/dropdown_group/components/workspaces_dropdown_group.vue @@ -148,11 +148,11 @@ export default { @@ -183,7 +183,11 @@ export default { @updateWorkspace="update(workspace.id, $event)" /> -
+

{{ $options.i18n.noWorkspacesMessage }}

diff --git a/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb b/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb index 1aaa8e8a2b4ce2..712366526ae60e 100644 --- a/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb +++ b/ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb @@ -97,7 +97,6 @@ click_button('Actions') # Asserts that all workspaces actions are visible - expect(page).to have_button('Restart') expect(page).to have_button('Stop') expect(page).to have_button('Terminate') diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index 676f8522fbc903..4e289a57652f95 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -135,7 +135,6 @@ # ASSERT ACTION BUTTONS ARE CORRECT FOR RUNNING STATE click_button 'Actions' - expect(page).to have_button('Restart') expect(page).to have_button('Stop') expect(page).to have_button('Terminate') diff --git a/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js b/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js index 3de867d9294fa1..a3f6794a48c363 100644 --- a/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js +++ b/ee/spec/frontend/workspaces/common/components/workspace_actions_spec.js @@ -35,7 +35,7 @@ describe('ee/workspaces/components/common/workspace_actions', () => { workspaceDisplayState | buttonsData ${ACTUAL.creationRequested} | ${[TERMINATE_BUTTON]} ${ACTUAL.starting} | ${[TERMINATE_BUTTON]} - ${ACTUAL.running} | ${[RESTART_BUTTON, STOP_BUTTON, TERMINATE_BUTTON]} + ${ACTUAL.running} | ${[STOP_BUTTON, TERMINATE_BUTTON]} ${ACTUAL.stopping} | ${[TERMINATE_BUTTON]} ${ACTUAL.stopped} | ${[START_BUTTON, TERMINATE_BUTTON]} ${ACTUAL.terminated} | ${[]} diff --git a/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js b/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js index 2179cdcce3849e..b9cc0558dce0be 100644 --- a/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js +++ b/ee/spec/frontend/workspaces/workspaces_dropdown_group/components/workspace_dropdown_item_spec.js @@ -1,10 +1,10 @@ -import { GlDisclosureDropdownItem } from '@gitlab/ui'; +import { GlLink } from '@gitlab/ui'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import { mockTracking } from 'helpers/tracking_helper'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; import WorkspaceStateIndicator from 'ee/workspaces/common/components/workspace_state_indicator.vue'; import WorkspaceActions from 'ee/workspaces/common/components/workspace_actions.vue'; -import { WORKSPACE_DESIRED_STATES } from 'ee/workspaces/dropdown_group/constants'; +import { WORKSPACE_DESIRED_STATES, WORKSPACE_STATES } from 'ee/workspaces/dropdown_group/constants'; import WorkspaceDropdownItem from 'ee/workspaces/dropdown_group/components/workspace_dropdown_item.vue'; import { calculateDisplayState } from 'ee/workspaces/common/services/calculate_display_state'; import { WORKSPACE } from '../../mock_data'; @@ -13,11 +13,13 @@ describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () let wrapper; let trackingSpy; - const createWrapper = () => { - // noinspection JSCheckFunctionSignatures - TODO: Address in https://gitlab.com/gitlab-org/gitlab/-/issues/437600 - wrapper = shallowMountExtended(WorkspaceDropdownItem, { + const createWrapper = ({ + props = { workspace: WORKSPACE }, + mountFn = shallowMountExtended, + } = {}) => { + wrapper = mountFn(WorkspaceDropdownItem, { propsData: { - workspace: WORKSPACE, + ...props, }, }); @@ -25,7 +27,7 @@ describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () }; const findWorkspaceStateIndicator = () => wrapper.findComponent(WorkspaceStateIndicator); const findWorkspaceActions = () => wrapper.findComponent(WorkspaceActions); - const findDropdownItem = () => wrapper.findComponent(GlDisclosureDropdownItem); + const findOpenWorkspaceLink = () => wrapper.findComponent(GlLink); describe('default', () => { const displayState = calculateDisplayState(WORKSPACE.actualState, WORKSPACE.desiredState); @@ -46,43 +48,66 @@ describe('workspaces/dropdown_group/components/workspace_dropdown_item.vue', () expect(wrapper.findComponent(TimeAgoTooltip).props('time')).toBe(WORKSPACE.createdAt); }); - it('passes workspace URL to the dropdown item', () => { - expect(findDropdownItem().props().item).toEqual({ - text: WORKSPACE.name, - href: WORKSPACE.url, - }); - }); - it('displays workspace actions', () => { expect(findWorkspaceActions().props().workspaceDisplayState).toEqual(displayState); }); }); - describe('when the dropdown item emits "action" event', () => { + describe('when workspace is running', () => { beforeEach(() => { - createWrapper(); - - findDropdownItem().vm.$emit('action'); + createWrapper({ + props: { + workspace: { + ...WORKSPACE, + desiredState: WORKSPACE_DESIRED_STATES.running, + actualState: WORKSPACE_STATES.running, + }, + }, + mountFn: mountExtended, + }); }); - it('tracks event', () => { - expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_consolidated_edit', { - label: 'workspace', + describe('when the dropdown item emits "action" event', () => { + beforeEach(() => { + findOpenWorkspaceLink().vm.$emit('click'); + }); + + it('tracks event', () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_consolidated_edit', { + label: 'workspace', + }); }); }); - }); - describe('when workspaces action is clicked', () => { - it('emits updateWorkspace event with the desiredState provided by the action', () => { - createWrapper(); + describe('when workspaces action is clicked', () => { + it('emits updateWorkspace event with the desiredState provided by the action', () => { + expect(wrapper.emitted('updateWorkspace')).toBe(undefined); - expect(wrapper.emitted('updateWorkspace')).toBe(undefined); + findWorkspaceActions().vm.$emit('click', WORKSPACE_DESIRED_STATES.running); + + expect(wrapper.emitted('updateWorkspace')).toEqual([ + [{ desiredState: WORKSPACE_DESIRED_STATES.running }], + ]); + }); + }); + + it.each` + component | selector + ${'workspace actions parent'} | ${() => findWorkspaceActions().element.parentElement} + ${'open workspace link'} | ${() => findOpenWorkspaceLink().element} + `('stops propagation of keydown event in $component component', ({ selector }) => { + const event = new KeyboardEvent('keydown', { + key: 'A', + keyCode: 65, + which: 64, + bubbles: true, + cancelable: true, + }); + const spy = jest.spyOn(event, 'stopPropagation'); - findWorkspaceActions().vm.$emit('click', WORKSPACE_DESIRED_STATES.running); + selector().dispatchEvent(event); - expect(wrapper.emitted('updateWorkspace')).toEqual([ - [{ desiredState: WORKSPACE_DESIRED_STATES.running }], - ]); + expect(spy).toHaveBeenCalled(); }); }); }); diff --git a/ee/spec/requests/remote_development/integration_spec.rb b/ee/spec/requests/remote_development/integration_spec.rb index 8bc6bbecf64d46..07a95d8fd882c3 100644 --- a/ee/spec/requests/remote_development/integration_spec.rb +++ b/ee/spec/requests/remote_development/integration_spec.rb @@ -218,6 +218,7 @@ def do_create_workspace(cluster_agent_id) expect(workspace.namespace).to eq("gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}") expect(workspace.url).to eq(URI::HTTPS.build({ host: "60001-#{workspace.name}.#{dns_zone}", + path: "/", query: { folder: "#{workspace_root}/#{workspace_project.path}" }.to_query diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d2a5e665da5fb2..559cb04b882f58 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -63169,6 +63169,9 @@ msgstr "" msgid "Workspaces|Create workspace" msgstr "" +msgid "Workspaces|Created" +msgstr "" + msgid "Workspaces|Creating" msgstr "" -- GitLab From 24893513b9535ee405410df956370b415704f80b Mon Sep 17 00:00:00 2001 From: Sam Beckham Date: Wed, 6 Nov 2024 13:33:54 +0000 Subject: [PATCH 3/5] Removes workspaces_table from vue 3 quarantine --- scripts/frontend/quarantined_vue3_specs.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/frontend/quarantined_vue3_specs.txt b/scripts/frontend/quarantined_vue3_specs.txt index d396f5a73ee9c9..5f349aa9831844 100644 --- a/scripts/frontend/quarantined_vue3_specs.txt +++ b/scripts/frontend/quarantined_vue3_specs.txt @@ -168,7 +168,6 @@ ee/spec/frontend/vulnerabilities/generic_report/types/list_graphql_spec.js ee/spec/frontend/vulnerabilities/related_issues_spec.js ee/spec/frontend/vulnerabilities/vulnerability_file_content_viewer_spec.js ee/spec/frontend/vulnerabilities/vulnerability_file_contents_spec.js -ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js spec/frontend/__helpers__/vue_test_utils_helper_spec.js spec/frontend/access_tokens/index_spec.js spec/frontend/add_context_commits_modal/components/add_context_commits_modal_spec.js -- GitLab From a0035268ab9fe7c7cf3f5db912138477c7807884 Mon Sep 17 00:00:00 2001 From: Enrique Alcantara Date: Wed, 6 Nov 2024 16:54:28 +0100 Subject: [PATCH 4/5] Fix rspec tests that do not add root path to URLs --- .../workspace_operations/create/main_integration_spec.rb | 1 + .../workspace_operations/create/workspace_creator_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb index 3a800384137be3..bb8bbdca477cd0 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb @@ -117,6 +117,7 @@ expect(workspace.workspaces_agent_config_version).to eq(expected_workspaces_agent_config_version) expect(workspace.url).to eq(URI::HTTPS.build({ host: "60001-#{workspace.name}.#{dns_zone}", + path: '/', query: { folder: "#{workspace_root}/#{project.path}" }.to_query diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb index 6711452afdf3bd..74296f1c572e14 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb @@ -66,7 +66,7 @@ it 'creates the workspace with the right url components' do expect(result).to be_ok_result do |message| message => { workspace: RemoteDevelopment::Workspace => workspace } - expected_url = "https://60001-#{workspace.name}.#{agent.unversioned_latest_workspaces_agent_config.dns_zone}" \ + expected_url = "https://60001-#{workspace.name}.#{agent.unversioned_latest_workspaces_agent_config.dns_zone}/" \ "?folder=%2Fprojects%2F#{project.path}" expect(workspace.url).to eq(expected_url) end -- GitLab From f60603cd44337dbcba8eb5c8db9387d13087a74d Mon Sep 17 00:00:00 2001 From: Enrique Alcantara Date: Thu, 7 Nov 2024 11:35:54 +0100 Subject: [PATCH 5/5] Code review feedback --- .../remote_development/workspaces_spec.rb | 221 +++++++++--------- .../workspaces_list/workspaces_table_spec.js | 15 +- 2 files changed, 116 insertions(+), 120 deletions(-) diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index 4e289a57652f95..f42a54eceb7b09 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -85,122 +85,115 @@ workspaces = RemoteDevelopment::Workspace.all.to_a expect(workspaces.length).to eq(1) workspace = workspaces[0] - within_workspace workspace do - # ASSERT ON NEW WORKSPACE IN LIST - expect(page).to have_content(workspace.name) - - # ASSERT WORKSPACE STATE BEFORE POLLING NEW STATES - expect_workspace_state_indicator('Creating') - - # ASSERT TERMINATE BUTTON IS AVAILABLE - click_button 'Actions' - expect(page).to have_button('Terminate') - - # CLOSE THE ACTIONS DROPDOWN - click_button 'Actions' - - additional_args_for_expected_config_to_apply = - build_additional_args_for_expected_config_to_apply( - network_policy_enabled: true, - dns_zone: agent.unversioned_latest_workspaces_agent_config.dns_zone, - namespace_path: group.path, - project_name: project.path - ) - - # SIMULATE FIRST POLL FROM AGENTK TO PICK UP NEW WORKSPACE - simulate_first_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # SIMULATE SECOND POLL FROM AGENTK TO UPDATE WORKSPACE TO RUNNING STATE - simulate_second_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS RUNNING STATE IN UI AND UPDATES URL - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::RUNNING) - expect(find_open_workspace_button).to have_text('Open workspace') - expect(find_open_workspace_button[:href]).to eq(workspace.url) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR RUNNING STATE - click_button 'Actions' - expect(page).to have_button('Stop') - expect(page).to have_button('Terminate') - - click_button 'Stop' - - # SIMULATE THIRD POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPING STATE - simulate_third_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS STOPPING STATE IN UI - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPING) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPING STATE - click_button 'Actions' - expect(page).to have_button('Terminate') - - # SIMULATE FOURTH POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPED STATE - simulate_fourth_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # ASSERT WORKSPACE SHOWS STOPPED STATE IN UI - expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPED) - - # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPED STATE - expect(page).to have_button('Start') - expect(page).to have_button('Terminate') - - # SIMULATE FIFTH POLL FROM AGENTK FOR PARTIAL RECONCILE TO SHOW NO RAILS_INFOS ARE SENT - simulate_fifth_poll do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end - - # SIMULATE SIXTH POLL FROM AGENTK FOR FULL RECONCILE TO SHOW ALL WORKSPACES ARE SENT IN RAILS_INFOS - simulate_sixth_poll( - workspace: workspace.reload, - **additional_args_for_expected_config_to_apply - ) do |workspace_agent_infos:, update_type:| - simulate_agentk_reconcile_post( - agent_token: agent_token, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - end + + # ASSERT ON NEW WORKSPACE IN LIST + expect(page).to have_content(workspace.name) + + # ASSERT WORKSPACE STATE BEFORE POLLING NEW STATES + expect_workspace_state_indicator('Creating') + + # ASSERT TERMINATE BUTTON IS AVAILABLE + click_button 'Actions' + expect(page).to have_button('Terminate') + + # CLOSE THE ACTIONS DROPDOWN + click_button 'Actions' + + additional_args_for_expected_config_to_apply = + build_additional_args_for_expected_config_to_apply( + network_policy_enabled: true, + dns_zone: agent.unversioned_latest_workspaces_agent_config.dns_zone, + namespace_path: group.path, + project_name: project.path + ) + + # SIMULATE FIRST POLL FROM AGENTK TO PICK UP NEW WORKSPACE + simulate_first_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # SIMULATE SECOND POLL FROM AGENTK TO UPDATE WORKSPACE TO RUNNING STATE + simulate_second_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS RUNNING STATE IN UI AND UPDATES URL + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::RUNNING) + expect(find_open_workspace_button).to have_text('Open workspace') + expect(find_open_workspace_button[:href]).to eq(workspace.url) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR RUNNING STATE + click_button 'Actions' + expect(page).to have_button('Stop') + expect(page).to have_button('Terminate') + + click_button 'Stop' + + # SIMULATE THIRD POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPING STATE + simulate_third_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS STOPPING STATE IN UI + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPING) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPING STATE + click_button 'Actions' + expect(page).to have_button('Terminate') + + # SIMULATE FOURTH POLL FROM AGENTK TO UPDATE WORKSPACE TO STOPPED STATE + simulate_fourth_poll(workspace: workspace.reload) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) + end + + # ASSERT WORKSPACE SHOWS STOPPED STATE IN UI + expect_workspace_state_indicator(RemoteDevelopment::WorkspaceOperations::States::STOPPED) + + # ASSERT ACTION BUTTONS ARE CORRECT FOR STOPPED STATE + expect(page).to have_button('Start') + expect(page).to have_button('Terminate') + + # SIMULATE FIFTH POLL FROM AGENTK FOR PARTIAL RECONCILE TO SHOW NO RAILS_INFOS ARE SENT + simulate_fifth_poll do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) end - end - def within_workspace(workspace) - page.within("div[data-testid=#{workspace.name}]") do - yield + # SIMULATE SIXTH POLL FROM AGENTK FOR FULL RECONCILE TO SHOW ALL WORKSPACES ARE SENT IN RAILS_INFOS + simulate_sixth_poll( + workspace: workspace.reload, + **additional_args_for_expected_config_to_apply + ) do |workspace_agent_infos:, update_type:| + simulate_agentk_reconcile_post( + agent_token: agent_token, + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + ) end end diff --git a/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js b/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js index 420e36b1749be8..8f38bc839d7618 100644 --- a/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js +++ b/ee/spec/frontend/workspaces/common/components/workspaces_list/workspaces_table_spec.js @@ -94,18 +94,21 @@ describe('workspaces/common/components/workspaces_list/workspaces_table.vue', () populateWorkspacesWithProjectDetails( USER_WORKSPACES_LIST_QUERY_RESULT.data.currentUser.workspaces.nodes, GET_PROJECTS_DETAILS_QUERY_RESULT.data.projects.nodes, - ).map((x) => { - const workspaceDisplayState = calculateDisplayState(x.actualState, x.desiredState); + ).map((workspace) => { + const workspaceDisplayState = calculateDisplayState( + workspace.actualState, + workspace.desiredState, + ); return { - nameText: x.name, - createdAt: x.createdAt, - terminates: x.name === 'workspace-1-1-idmi02' ? 'in 54 minutes' : 'in 2 days', + nameText: workspace.name, + createdAt: workspace.createdAt, + terminates: workspace.name === 'workspace-1-1-idmi02' ? 'in 54 minutes' : 'in 2 days', workspaceActionsProps: { workspaceDisplayState, }, openWorkspaceButtonProps: { workspaceDisplayState, - workspaceUrl: x.url, + workspaceUrl: workspace.url, }, stateIndicatorProps: { workspaceDisplayState, -- GitLab