[Refactor] Create Fragment to encapsulate Workspaces fields
Issue: [Refactor] Create Fragment to encapsulate commo... (#443619 - closed)
What does this MR do and why?
Implements Graphql cleanup described in [Refactor] Create Fragment to encapsulate commo... (#443619 - closed)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
How to set up and validate locally
All tests should pass.
Related to #443619 (closed)
Merge request reports
Activity
changed milestone to %Backlog
assigned to @cwoolley-gitlab
mentioned in issue #443619 (closed)
added docs-only label
mentioned in commit 9a995025
added 1 commit
- 9a995025 - Extract common workspace fields to GraphQL fragment
removed docs-only label
4 Warnings 
1b936f12: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 1b936f12: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. 1b936f12: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
1 Message 
CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelogtrailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Category Reviewer Maintainer frontend @lcallahan
(UTC-6, 1 hour ahead of author)
@mfluharty (UTC-6, 1 hour ahead of author)
Please check reviewer's status!
-
Reviewer is available!
-
Reviewer is unavailable!
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-reviewjob that generated this comment.Generated by
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 0cf1c9f0 and 1b936f12
Entrypoint / Name Size before Size after Diff Diff in percent average 4.25 MB 4.25 MB - 0.0 % mainChunk 3.23 MB 3.23 MB - 0.0 %
Note: We do not have exact data for 0cf1c9f0. So we have used data from: bdd554c8.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun thebundle-size-reviewjob
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.Please look at the full report for more details
Read more about how this report works.
Generated by
mentioned in commit 87f64c9b
added 14 commits
- 9a995025...2638be18 - 4 earlier commits
- 7a5b8c80 - More review feedback
- 1c96b295 - Add test coverage
- 063d9415 - Improve display of devfile field in workspaces list
- 7da4795f - Review feedback - refactor focus
- b736e093 - Review feedback - fixes for workspaces_table.vue
- c167b06e - Review feedback - test cleanup
- 99393122 - Review feedback - create component and tests cleanup
- 98937ce4 - Review feedback from technical writer
- 90bf68ae - Fix failing spec
- 87f64c9b - Extract common workspace fields to GraphQL fragment
Toggle commit list@pslaughter I could use some help on this one.
When I run
yarn jest ee/spec/frontend/remote_development, there's many failures due to things being not present orundefined, but it's unclear to me what or why, seems to be something related to Apollo.However, when I run the same changes in GraphiQL, they seem to work fine:
Fragments and query:
fragment WorkspaceFields on Workspace { id name namespace projectId desiredState actualState url devfileRef devfilePath devfileWebUrl createdAt } fragment PageInfo on PageInfo { hasNextPage hasPreviousPage startCursor endCursor } fragment WorkspaceConnectionFragment on WorkspaceConnection { nodes { ...WorkspaceFields } pageInfo { ...PageInfo } } query workspaces { workspaces(actualStates: ["Terminated"]) { ...WorkspaceConnectionFragment } }Response:
{ "data": { "workspaces": { "nodes": [ { "id": "gid://gitlab/RemoteDevelopment::Workspace/111", "name": "workspace-9-1-nn5brz", "namespace": "gl-rd-ns-9-1-nn5brz", "projectId": "gid://gitlab/Project/2", "desiredState": "Terminated", "actualState": "Terminated", "url": "https://60001-workspace-9-1-nn5brz.workspaces.poweredge.mindlikewater.net?folder=%2Fprojects%2Fgitlab-shell", "devfileRef": "main", "devfilePath": ".devfile.yaml", "devfileWebUrl": "http://gdk.test:3000/gitlab-org/gitlab-shell/-/blob/main/.devfile.yaml", "createdAt": "2024-02-23T08:39:02Z" } ], "pageInfo": { "hasNextPage": false, "hasPreviousPage": false, "startCursor": "eyJpZCI6IjExMSJ9", "endCursor": "eyJpZCI6IjExMSJ9" } } } }And running the app on this branch also seems to work fine, all
graphqlrequests work and the list/create page work.So it seems to just be a problem with the tests.
- A deleted user
added backend documentation labels
requested review from @pslaughter
Resolved by Paul Slaughtertodo: @cwoolley-gitlab it's not obvious at all but the issue here comes from missing
__typenamein our mock responses. I'm guessing thatapollowon't apply the fragment on our mock data since the data's__typename(which wasundefined) doesn't match theon Workspacepart of the fragment.I believe that
__typenameis always included in production queries/responses, so this should only be a test-environment issue.Here's a patch based off of
masterthat introducesFragmentand updates ourmock_datato include__typename:diff --git a/ee/app/assets/javascripts/remote_development/graphql/fragments/workspace_item.fragment.graphql b/ee/app/assets/javascripts/remote_development/graphql/fragments/workspace_item.fragment.graphql new file mode 100644 index 000000000000..7fa49723f9fa --- /dev/null +++ b/ee/app/assets/javascripts/remote_development/graphql/fragments/workspace_item.fragment.graphql @@ -0,0 +1,13 @@ +fragment WorkspaceItem on Workspace { + id + name + namespace + projectId + desiredState + actualState + url + devfileRef + devfilePath + devfileWebUrl + createdAt +} diff --git a/ee/app/assets/javascripts/remote_development/graphql/mutations/workspace_create.mutation.graphql b/ee/app/assets/javascripts/remote_development/graphql/mutations/workspace_create.mutation.graphql index 7504a3985c81..5439a6571760 100644 --- a/ee/app/assets/javascripts/remote_development/graphql/mutations/workspace_create.mutation.graphql +++ b/ee/app/assets/javascripts/remote_development/graphql/mutations/workspace_create.mutation.graphql @@ -1,17 +1,9 @@ +#import "../fragments/workspace_item.fragment.graphql" + mutation workspaceCreate($input: WorkspaceCreateInput!) { workspaceCreate(input: $input) { workspace { - id - name - namespace - projectId - desiredState - actualState - url - devfileRef - devfilePath - devfileWebUrl - createdAt + ...WorkspaceItem } errors } diff --git a/ee/app/assets/javascripts/remote_development/graphql/queries/agent_workspaces_list.query.graphql b/ee/app/assets/javascripts/remote_development/graphql/queries/agent_workspaces_list.query.graphql index 918a7b788fcc..be491029b8bf 100644 --- a/ee/app/assets/javascripts/remote_development/graphql/queries/agent_workspaces_list.query.graphql +++ b/ee/app/assets/javascripts/remote_development/graphql/queries/agent_workspaces_list.query.graphql @@ -1,4 +1,5 @@ #import "~/graphql_shared/fragments/page_info.fragment.graphql" +#import "../fragments/workspace_item.fragment.graphql" query agentWorkspaces( $first: Int @@ -13,17 +14,7 @@ query agentWorkspaces( id workspaces(first: $first, before: $before, after: $after) { nodes { - id - name - namespace - projectId - desiredState - actualState - url - devfileRef - devfilePath - devfileWebUrl - createdAt + ...WorkspaceItem } pageInfo { ...PageInfo diff --git a/ee/app/assets/javascripts/remote_development/graphql/queries/user_workspaces_list.query.graphql b/ee/app/assets/javascripts/remote_development/graphql/queries/user_workspaces_list.query.graphql index 76def1b35cbe..54e5d535b146 100644 --- a/ee/app/assets/javascripts/remote_development/graphql/queries/user_workspaces_list.query.graphql +++ b/ee/app/assets/javascripts/remote_development/graphql/queries/user_workspaces_list.query.graphql @@ -1,4 +1,5 @@ #import "~/graphql_shared/fragments/page_info.fragment.graphql" +#import "../fragments/workspace_item.fragment.graphql" query userWorkspacesList( $first: Int @@ -18,16 +19,7 @@ query userWorkspacesList( ) { nodes { id - name - namespace - projectId - desiredState - actualState - url - devfileRef - devfilePath - devfileWebUrl - createdAt + ...WorkspaceItem } pageInfo { ...PageInfo diff --git a/ee/spec/frontend/remote_development/mock_data/index.js b/ee/spec/frontend/remote_development/mock_data/index.js index 974b16ad9e46..c31bb4ee4c93 100644 --- a/ee/spec/frontend/remote_development/mock_data/index.js +++ b/ee/spec/frontend/remote_development/mock_data/index.js @@ -3,6 +3,7 @@ import { TEST_HOST } from 'helpers/test_constants'; import { WORKSPACE_DESIRED_STATES, WORKSPACE_STATES } from 'ee/remote_development/constants'; export const WORKSPACE = { + __typename: 'Workspace', id: 1, name: 'Workspace 1', namespace: 'Namespace', @@ -32,6 +33,7 @@ export const USER_WORKSPACES_LIST_QUERY_RESULT = { workspaces: { nodes: [ { + __typename: 'Workspace', id: 'gid://gitlab/RemoteDevelopment::Workspace/2', name: 'workspace-1-1-idmi02', namespace: 'gl-rd-ns-1-1-idmi02', @@ -45,6 +47,7 @@ export const USER_WORKSPACES_LIST_QUERY_RESULT = { createdAt: '2023-04-29T18:24:34Z', }, { + __typename: 'Workspace', id: 'gid://gitlab/RemoteDevelopment::Workspace/1', name: 'workspace-1-1-rfu27q', namespace: 'gl-rd-ns-1-1-rfu27q', @@ -95,6 +98,7 @@ export const AGENT_WORKSPACES_LIST_QUERY_RESULT = { workspaces: { nodes: [ { + __typename: 'Workspace', id: 'gid://gitlab/RemoteDevelopment::Workspace/2', name: 'workspace-1-1-idmi02', namespace: 'gl-rd-ns-1-1-idmi02', @@ -109,6 +113,7 @@ export const AGENT_WORKSPACES_LIST_QUERY_RESULT = { createdAt: '2023-04-29T18:24:34Z', }, { + __typename: 'Workspace', id: 'gid://gitlab/RemoteDevelopment::Workspace/1', name: 'workspace-1-1-rfu27q', namespace: 'gl-rd-ns-1-1-rfu27q', @@ -307,6 +312,7 @@ export const WORKSPACE_UPDATE_MUTATION_RESULT = { workspaceUpdate: { errors: [], workspace: { + __typename: 'Workspace', id: WORKSPACE.id, actualState: WORKSPACE_STATES.running, desiredState: WORKSPACE_DESIRED_STATES.restartRequested,Could you please resolve this MR with these changes
WorkspaceItemto represent that these are the fields we expect in each item of ourWorkspaceList. WDYT (I'm not super attached to the name)?- Last reply by Chad Woolley
removed review request for @pslaughter
mentioned in commit 55be04af
added 3852 commits
-
c102da7a...000a47ea - 3850 commits from branch
master - 55be04af - Extract common workspace fields to GraphQL fragment
- a49b72f2 - apply patch
-
c102da7a...000a47ea - 3850 commits from branch
requested review from @pslaughter
I don't know what all these build failures in the
rspec-all frontend_fixturejobs, e.g. https://gitlab.com/gitlab-org/gitlab/-/jobs/6530828340They seem unrelated/flakes, but still happened even after a fresh rebase onto
master.mentioned in issue #454227 (closed)
- Resolved by Paul Slaughter
Thanks for working on this @cwoolley-gitlab! There's a small TODO based on a typo then I think we should be good to go. Back to you!
- Last reply by Chad Woolley
removed review request for @pslaughter
requested review from @pslaughter
-
added pipeline:mr-approved label
Resolved by Paul Slaughter
@pslaughter, thanks for approving this merge request.This is the first time the merge request has been approved. To ensure we don't only run predictive pipelines, and we don't break
master, a new pipeline will be started shortly.Please wait for the pipeline to start before resolving this discussion and set auto-merge for the new pipeline. See merging a merge request for more details.
enabled an automatic merge when the pipeline for d449625e succeeds
E2E Test Result Summary
allure-report-publishergenerated test report!e2e-test-on-gdk:
expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Govern | 66 | 0 | 0 | 0 | 66 | ✅ | | Data Stores | 31 | 0 | 0 | 0 | 31 | ✅ | | Plan | 51 | 0 | 2 | 0 | 53 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Create | 77 | 0 | 9 | 0 | 86 | ✅ | | Verify | 35 | 0 | 1 | 0 | 36 | ✅ | | Monitor | 7 | 0 | 0 | 0 | 7 | ✅ | | Package | 24 | 0 | 6 | 0 | 30 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 298 | 0 | 19 | 0 | 317 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+e2e-package-and-test:
expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 570 | 0 | 66 | 10 | 636 | ✅ | | Plan | 8 | 0 | 0 | 0 | 8 | ✅ | | Govern | 6 | 0 | 0 | 0 | 6 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Data Stores | 4 | 0 | 0 | 0 | 4 | ✅ | | Package | 0 | 0 | 2 | 0 | 2 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 596 | 0 | 68 | 10 | 664 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+mentioned in commit ea0341fe
mentioned in commit c62a430d
added workflowstaging-canary label and removed workflowrefinement label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
added workflowpost-deploy-db-staging label and removed workflowproduction label
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
added pipelinetier-3 label
