From 5940e90217bee3495e22f68eaf3cf8cd47fa56e2 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 12 Sep 2024 09:42:59 +0100
Subject: [PATCH] Improved perceived performance on the new merge request
dashboard
The database query to get the count for the number of merge requests in a list
can take much longer than the query to get the merge requests.
With this change we request the count separately so that we can load the merge requests
quickly and display them without waiting for the count query to finish.
---
.../components/merge_requests_query.vue | 31 ++++++++++++++++---
.../queries/assignee.query.graphql | 1 -
.../queries/assignee_count.query.graphql | 20 ++++++++++++
.../assignee_or_reviewer.query.graphql | 1 -
.../assignee_or_reviewer_count.query.graphql | 18 +++++++++++
.../queries/reviewer.query.graphql | 1 -
.../queries/reviewer_count.query.graphql | 18 +++++++++++
.../components/app_spec.js | 19 ++++++++++--
.../components/merge_requests_query_spec.js | 28 +++++++++++++----
9 files changed, 121 insertions(+), 16 deletions(-)
create mode 100644 app/assets/javascripts/merge_request_dashboard/queries/assignee_count.query.graphql
create mode 100644 app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer_count.query.graphql
create mode 100644 app/assets/javascripts/merge_request_dashboard/queries/reviewer_count.query.graphql
diff --git a/app/assets/javascripts/merge_request_dashboard/components/merge_requests_query.vue b/app/assets/javascripts/merge_request_dashboard/components/merge_requests_query.vue
index 573b7afcf325..34761f9d0468 100644
--- a/app/assets/javascripts/merge_request_dashboard/components/merge_requests_query.vue
+++ b/app/assets/javascripts/merge_request_dashboard/components/merge_requests_query.vue
@@ -1,21 +1,27 @@
<script>
import reviewerQuery from '../queries/reviewer.query.graphql';
+import reviewerCountQuery from '../queries/reviewer_count.query.graphql';
import assigneeQuery from '../queries/assignee.query.graphql';
+import assigneeCountQuery from '../queries/assignee_count.query.graphql';
import assigneeOrReviewerQuery from '../queries/assignee_or_reviewer.query.graphql';
+import assigneeOrReviewerCountQuery from '../queries/assignee_or_reviewer_count.query.graphql';
const PER_PAGE = 20;
const QUERIES = {
- reviewRequestedMergeRequests: reviewerQuery,
- assignedMergeRequests: assigneeQuery,
- assigneeOrReviewerMergeRequests: assigneeOrReviewerQuery,
+ reviewRequestedMergeRequests: { dataQuery: reviewerQuery, countQuery: reviewerCountQuery },
+ assignedMergeRequests: { dataQuery: assigneeQuery, countQuery: assigneeCountQuery },
+ assigneeOrReviewerMergeRequests: {
+ dataQuery: assigneeOrReviewerQuery,
+ countQuery: assigneeOrReviewerCountQuery,
+ },
};
export default {
apollo: {
mergeRequests: {
query() {
- return QUERIES[this.query];
+ return QUERIES[this.query].dataQuery;
},
update(d) {
return d.currentUser?.mergeRequests || {};
@@ -30,6 +36,20 @@ export default {
this.error = true;
},
},
+ count: {
+ query() {
+ return QUERIES[this.query].countQuery;
+ },
+ update(d) {
+ return d.currentUser?.mergeRequests?.count;
+ },
+ variables() {
+ return {
+ ...this.variables,
+ perPage: PER_PAGE,
+ };
+ },
+ },
},
props: {
query: {
@@ -44,6 +64,7 @@ export default {
data() {
return {
mergeRequests: null,
+ count: null,
error: false,
};
},
@@ -69,7 +90,7 @@ export default {
render() {
return this.$scopedSlots.default({
mergeRequests: this.mergeRequests?.nodes || [],
- count: this.mergeRequests ? this.mergeRequests.count : null,
+ count: this.count,
hasNextPage: this.hasNextPage,
loadMore: this.loadMore,
loading: this.isLoading,
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/assignee.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/assignee.query.graphql
index 25dc21a07b27..e7361721e649 100644
--- a/app/assets/javascripts/merge_request_dashboard/queries/assignee.query.graphql
+++ b/app/assets/javascripts/merge_request_dashboard/queries/assignee.query.graphql
@@ -22,7 +22,6 @@ query requestingReview(
after: $afterCursor
sort: UPDATED_DESC
) {
- count
pageInfo {
...PageInfo
}
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/assignee_count.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/assignee_count.query.graphql
new file mode 100644
index 000000000000..d533a157a486
--- /dev/null
+++ b/app/assets/javascripts/merge_request_dashboard/queries/assignee_count.query.graphql
@@ -0,0 +1,20 @@
+query requestingReviewCount(
+ $state: MergeRequestState = opened
+ $reviewState: MergeRequestReviewState
+ $reviewStates: [MergeRequestReviewState!]
+ $reviewerWildcardId: ReviewerWildcardId
+ $mergedAfter: Time
+) {
+ currentUser {
+ id
+ mergeRequests: assignedMergeRequests(
+ state: $state
+ reviewState: $reviewState
+ reviewStates: $reviewStates
+ reviewerWildcardId: $reviewerWildcardId
+ mergedAfter: $mergedAfter
+ ) {
+ count
+ }
+ }
+}
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer.query.graphql
index 5a743656a194..a0b65e70867d 100644
--- a/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer.query.graphql
+++ b/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer.query.graphql
@@ -20,7 +20,6 @@ query assigneeOrReviewer(
after: $afterCursor
sort: UPDATED_DESC
) {
- count
pageInfo {
...PageInfo
}
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer_count.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer_count.query.graphql
new file mode 100644
index 000000000000..c27f0a159fb4
--- /dev/null
+++ b/app/assets/javascripts/merge_request_dashboard/queries/assignee_or_reviewer_count.query.graphql
@@ -0,0 +1,18 @@
+query assigneeOrReviewerCount(
+ $state: MergeRequestState = opened
+ $assignedReviewStates: [MergeRequestReviewState!]
+ $reviewerReviewStates: [MergeRequestReviewState!]
+ $mergedAfter: Time
+) {
+ currentUser {
+ id
+ mergeRequests: assigneeOrReviewerMergeRequests(
+ state: $state
+ assignedReviewStates: $assignedReviewStates
+ reviewerReviewStates: $reviewerReviewStates
+ mergedAfter: $mergedAfter
+ ) {
+ count
+ }
+ }
+}
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/reviewer.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/reviewer.query.graphql
index fbdfcb46c06f..e0c74ffed5ac 100644
--- a/app/assets/javascripts/merge_request_dashboard/queries/reviewer.query.graphql
+++ b/app/assets/javascripts/merge_request_dashboard/queries/reviewer.query.graphql
@@ -20,7 +20,6 @@ query reviewRequests(
after: $afterCursor
sort: UPDATED_DESC
) {
- count
pageInfo {
...PageInfo
}
diff --git a/app/assets/javascripts/merge_request_dashboard/queries/reviewer_count.query.graphql b/app/assets/javascripts/merge_request_dashboard/queries/reviewer_count.query.graphql
new file mode 100644
index 000000000000..c7232688c8dd
--- /dev/null
+++ b/app/assets/javascripts/merge_request_dashboard/queries/reviewer_count.query.graphql
@@ -0,0 +1,18 @@
+query reviewRequestsCount(
+ $state: MergeRequestState = opened
+ $reviewState: MergeRequestReviewState
+ $reviewStates: [MergeRequestReviewState!]
+ $mergedAfter: Time
+) {
+ currentUser {
+ id
+ mergeRequests: reviewRequestedMergeRequests(
+ state: $state
+ reviewState: $reviewState
+ reviewStates: $reviewStates
+ mergedAfter: $mergedAfter
+ ) {
+ count
+ }
+ }
+}
diff --git a/spec/frontend/merge_request_dashboard/components/app_spec.js b/spec/frontend/merge_request_dashboard/components/app_spec.js
index cbd98607e26e..81e0b97363a4 100644
--- a/spec/frontend/merge_request_dashboard/components/app_spec.js
+++ b/spec/frontend/merge_request_dashboard/components/app_spec.js
@@ -8,6 +8,7 @@ import MergeRequestsQuery from '~/merge_request_dashboard/components/merge_reque
import CollapsibleSection from '~/merge_request_dashboard/components/collapsible_section.vue';
import MergeRequest from '~/merge_request_dashboard/components/merge_request.vue';
import assigneeQuery from '~/merge_request_dashboard/queries/assignee.query.graphql';
+import assigneeCountQuery from '~/merge_request_dashboard/queries/assignee_count.query.graphql';
import { createMockMergeRequest } from '../mock_data';
Vue.use(VueApollo);
@@ -27,7 +28,6 @@ describe('Merge requests app component', () => {
currentUser: {
id: 1,
mergeRequests: {
- count: 1,
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
@@ -41,7 +41,22 @@ describe('Merge requests app component', () => {
},
});
const apolloProvider = createMockApollo(
- [[assigneeQuery, assigneeQueryMock]],
+ [
+ [assigneeQuery, assigneeQueryMock],
+ [
+ assigneeCountQuery,
+ jest.fn().mockResolvedValue({
+ data: {
+ currentUser: {
+ id: 1,
+ mergeRequests: {
+ count: 1,
+ },
+ },
+ },
+ }),
+ ],
+ ],
{},
{ typePolicies: { Query: { fields: { currentUser: { merge: false } } } } },
);
diff --git a/spec/frontend/merge_request_dashboard/components/merge_requests_query_spec.js b/spec/frontend/merge_request_dashboard/components/merge_requests_query_spec.js
index 4b1fc388c9b8..99cf15c0bab2 100644
--- a/spec/frontend/merge_request_dashboard/components/merge_requests_query_spec.js
+++ b/spec/frontend/merge_request_dashboard/components/merge_requests_query_spec.js
@@ -5,7 +5,9 @@ import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
import MergeRequestQuery from '~/merge_request_dashboard/components/merge_requests_query.vue';
import reviewerQuery from '~/merge_request_dashboard/queries/reviewer.query.graphql';
+import reviewerCountQuery from '~/merge_request_dashboard/queries/reviewer_count.query.graphql';
import assigneeQuery from '~/merge_request_dashboard/queries/assignee.query.graphql';
+import assigneeCountQuery from '~/merge_request_dashboard/queries/assignee_count.query.graphql';
import { createMockMergeRequest } from '../mock_data';
Vue.use(VueApollo);
@@ -23,7 +25,6 @@ describe('Merge requests query component', () => {
currentUser: {
id: 1,
mergeRequests: {
- count: 0,
pageInfo: {
__typename: 'PageInfo',
hasNextPage: false,
@@ -41,7 +42,6 @@ describe('Merge requests query component', () => {
currentUser: {
id: 1,
mergeRequests: {
- count: 0,
pageInfo: {
hasNextPage: false,
hasPreviousPage: false,
@@ -54,10 +54,26 @@ describe('Merge requests query component', () => {
},
},
});
- const apolloProvider = createMockApollo([
- [reviewerQuery, reviewerQueryMock],
- [assigneeQuery, assigneeQueryMock],
- ]);
+ const apolloProvider = createMockApollo(
+ [
+ [reviewerQuery, reviewerQueryMock],
+ [assigneeQuery, assigneeQueryMock],
+ [
+ reviewerCountQuery,
+ jest
+ .fn()
+ .mockResolvedValue({ data: { currentUser: { id: 1, mergeRequests: { count: 1 } } } }),
+ ],
+ [
+ assigneeCountQuery,
+ jest
+ .fn()
+ .mockResolvedValue({ data: { currentUser: { id: 1, mergeRequests: { count: 1 } } } }),
+ ],
+ ],
+ {},
+ { typePolicies: { Query: { fields: { currentUser: { merge: false } } } } },
+ );
slotSpy = jest.fn();
--
GitLab