From ce48878131c33148aa2baa65744ee76dca1f54c9 Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 09:38:47 +0100 Subject: [PATCH 1/9] Renamed page to match naming --- src/pages/{IndividualReport.vue => StudentReport.vue} | 0 src/router/index.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/pages/{IndividualReport.vue => StudentReport.vue} (100%) diff --git a/src/pages/IndividualReport.vue b/src/pages/StudentReport.vue similarity index 100% rename from src/pages/IndividualReport.vue rename to src/pages/StudentReport.vue diff --git a/src/router/index.js b/src/router/index.js index a0bab8664..9862ef04f 100644 --- a/src/router/index.js +++ b/src/router/index.js @@ -353,7 +353,7 @@ const routes = [ path: APP_ROUTES.STUDENT_REPORT, name: 'StudentReport', props: true, - component: () => import('../pages/IndividualReport.vue'), + component: () => import('../pages/StudentReport.vue'), meta: { pageTitle: 'Student Score Report', requireAdmin: true }, }, { From 674bb5a1b57feb36c82e81673b85685611289dae Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 09:58:02 +0100 Subject: [PATCH 2/9] Adapt useUserDataQuery to optionally fetch by manual user ID --- src/components/views/OfflineSettings.vue | 2 +- src/components/views/UserInfoView.vue | 2 +- src/composables/queries/useUserDataQuery.js | 11 ++++--- .../queries/useUserDataQuery.test.js | 31 +++++++++++++++++-- src/pages/HomeParticipant.vue | 2 +- src/pages/HomeSelector.vue | 2 +- src/pages/StudentReport.vue | 25 +++++++-------- 7 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/components/views/OfflineSettings.vue b/src/components/views/OfflineSettings.vue index bdfe4e279..0e4a6e1fa 100644 --- a/src/components/views/OfflineSettings.vue +++ b/src/components/views/OfflineSettings.vue @@ -187,7 +187,7 @@ const init = () => { const { mutate: updateUser } = useUpdateUserMutation(); -const { data: userData, isLoading: isLoadingUserData } = useUserDataQuery({ +const { data: userData, isLoading: isLoadingUserData } = useUserDataQuery(null, { enabled: initialized, }); diff --git a/src/components/views/UserInfoView.vue b/src/components/views/UserInfoView.vue index 54caa6ffb..6464624d5 100644 --- a/src/components/views/UserInfoView.vue +++ b/src/components/views/UserInfoView.vue @@ -73,7 +73,7 @@ onMounted(() => { // +---------+ // | Queries | // +---------+ -const { data: userData } = useUserDataQuery({ +const { data: userData } = useUserDataQuery(null, { enabled: initialized, }); diff --git a/src/composables/queries/useUserDataQuery.js b/src/composables/queries/useUserDataQuery.js index e9e9cae20..a91e36df2 100644 --- a/src/composables/queries/useUserDataQuery.js +++ b/src/composables/queries/useUserDataQuery.js @@ -5,23 +5,26 @@ import { computeQueryOverrides } from '@/helpers/computeQueryOverrides'; import { fetchDocById } from '@/helpers/query/utils'; import { USER_DATA_QUERY_KEY } from '@/constants/queryKeys'; import { FIRESTORE_COLLECTIONS } from '@/constants/firebase'; +import { computed } from 'vue'; /** * User profile data query. * + * @param {string|undefined|null} userId – The user ID to fetch, set to a falsy value to fetch the current user. * @param {QueryOptions|undefined} queryOptions – Optional TanStack query options. * @returns {UseQueryResult} The TanStack query result. */ -const useUserDataQuery = (queryOptions = undefined) => { +const useUserDataQuery = (userId = undefined, queryOptions = undefined) => { const authStore = useAuthStore(); const { roarUid, userQueryKeyIndex } = storeToRefs(authStore); - const queryConditions = [() => !!roarUid.value]; + const uid = computed(() => userId || roarUid.value); + const queryConditions = [() => !!uid.value]; const { isQueryEnabled, options } = computeQueryOverrides(queryConditions, queryOptions); return useQuery({ - queryKey: [USER_DATA_QUERY_KEY, roarUid.value, userQueryKeyIndex.value], - queryFn: () => fetchDocById(FIRESTORE_COLLECTIONS.USERS, roarUid.value), + queryKey: [USER_DATA_QUERY_KEY, uid.value, userQueryKeyIndex.value], + queryFn: () => fetchDocById(FIRESTORE_COLLECTIONS.USERS, uid.value), enabled: isQueryEnabled, ...options, }); diff --git a/src/composables/queries/useUserDataQuery.test.js b/src/composables/queries/useUserDataQuery.test.js index a05392ad4..667dde6b1 100644 --- a/src/composables/queries/useUserDataQuery.test.js +++ b/src/composables/queries/useUserDataQuery.test.js @@ -57,6 +57,31 @@ describe('useUserDataQuery', () => { expect(fetchDocById).toHaveBeenCalledWith('users', mockUserId); }); + it('should allow the use of a manual user ID', async () => { + const mockUserId = nanoid(); + const mockStudentUserId = nanoid(); + + const authStore = useAuthStore(piniaInstance); + authStore.roarUid = mockUserId; + authStore.userQueryKeyIndex = 1; + + vi.spyOn(VueQuery, 'useQuery'); + + withSetup(() => useUserDataQuery(mockStudentUserId), { + plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], + }); + + expect(VueQuery.useQuery).toHaveBeenCalledWith({ + queryKey: ['user', mockStudentUserId, 1], + queryFn: expect.any(Function), + enabled: expect.objectContaining({ + _value: true, + }), + }); + + expect(fetchDocById).toHaveBeenCalledWith('users', mockStudentUserId); + }); + it('should correctly control the enabled state of the query', async () => { const mockUserId = nanoid(); @@ -70,7 +95,7 @@ describe('useUserDataQuery', () => { enabled: enableQuery, }; - withSetup(() => useUserDataQuery(queryOptions), { + withSetup(() => useUserDataQuery(null, queryOptions), { plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], }); @@ -100,7 +125,7 @@ describe('useUserDataQuery', () => { const queryOptions = { enabled: true }; - withSetup(() => useUserDataQuery(queryOptions), { + withSetup(() => useUserDataQuery(null, queryOptions), { plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], }); @@ -128,7 +153,7 @@ describe('useUserDataQuery', () => { const queryOptions = { enabled: true }; - withSetup(() => useUserDataQuery(queryOptions), { + withSetup(() => useUserDataQuery(null, queryOptions), { plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], }); diff --git a/src/pages/HomeParticipant.vue b/src/pages/HomeParticipant.vue index 9013c8b5f..3a2653087 100644 --- a/src/pages/HomeParticipant.vue +++ b/src/pages/HomeParticipant.vue @@ -148,7 +148,7 @@ const { isLoading: isLoadingUserData, isFetching: isFetchingUserData, data: userData, -} = useUserDataQuery({ +} = useUserDataQuery(null, { enabled: initialized, }); diff --git a/src/pages/HomeSelector.vue b/src/pages/HomeSelector.vue index bc6ba61c9..02f92041d 100644 --- a/src/pages/HomeSelector.vue +++ b/src/pages/HomeSelector.vue @@ -67,7 +67,7 @@ unsubscribe = authStore.$subscribe(async (mutation, state) => { if (state.roarfirekit.restConfig) init(); }); -const { isLoading: isLoadingUserData, data: userData } = useUserDataQuery({ +const { isLoading: isLoadingUserData, data: userData } = useUserDataQuery(null, { enabled: initialized, }); diff --git a/src/pages/StudentReport.vue b/src/pages/StudentReport.vue index ba0dde3ab..2d41f7326 100644 --- a/src/pages/StudentReport.vue +++ b/src/pages/StudentReport.vue @@ -174,19 +174,20 @@ From da98e57299bb5bd83f0a1f201c5008c8bc5d2ca4 Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 17:06:02 +0100 Subject: [PATCH 6/9] Add useUserAdministrationAssignmentsQuery unit tests --- ...UserAdministrationAssignmentsQuery.test.js | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 src/composables/queries/useUserAdministrationAssignmentsQuery.test.js diff --git a/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js b/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js new file mode 100644 index 000000000..19f23d232 --- /dev/null +++ b/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js @@ -0,0 +1,145 @@ +import { ref, nextTick } from 'vue'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { createTestingPinia } from '@pinia/testing'; +import * as VueQuery from '@tanstack/vue-query'; +import { nanoid } from 'nanoid'; +import { withSetup } from '@/test-support/withSetup.js'; +import { useAuthStore } from '@/store/auth'; +import { fetchDocById } from '@/helpers/query/utils'; +import useUserAdministrationAssignmentsQuery from './useUserAdministrationAssignmentsQuery'; + +vi.mock('@/helpers/query/utils', () => ({ + fetchDocById: vi.fn().mockImplementation(() => []), +})); + +vi.mock('@tanstack/vue-query', async (getModule) => { + const original = await getModule(); + return { + ...original, + useQuery: vi.fn().mockImplementation(original.useQuery), + }; +}); + +describe('useUserAdministrationAssignmentsQuery', () => { + let piniaInstance; + let queryClient; + + beforeEach(() => { + piniaInstance = createTestingPinia(); + queryClient = new VueQuery.QueryClient(); + }); + + afterEach(() => { + queryClient?.clear(); + }); + + it('should call query with correct parameters', () => { + const mockUserId = nanoid(); + const mockAdministrationId = nanoid(); + + vi.spyOn(VueQuery, 'useQuery'); + + withSetup(() => useUserAdministrationAssignmentsQuery(mockUserId, mockAdministrationId), { + plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], + }); + + expect(VueQuery.useQuery).toHaveBeenCalledWith({ + queryKey: ['user-administration-assignments', mockUserId, mockAdministrationId], + queryFn: expect.any(Function), + enabled: expect.objectContaining({ + _value: true, + }), + }); + + expect(fetchDocById).toHaveBeenCalledWith('users', `${mockUserId}/assignments/${mockAdministrationId}`); + }); + + it('should correctly control the enabled state of the query', async () => { + const mockUserId = nanoid(); + const mockAdministrationId = nanoid(); + + const enableQuery = ref(false); + + vi.spyOn(VueQuery, 'useQuery'); + + const queryOptions = { + enabled: enableQuery, + }; + + withSetup(() => useUserAdministrationAssignmentsQuery(mockUserId, mockAdministrationId, queryOptions), { + plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], + }); + + expect(VueQuery.useQuery).toHaveBeenCalledWith({ + queryKey: ['user-administration-assignments', mockUserId, mockAdministrationId], + queryFn: expect.any(Function), + enabled: expect.objectContaining({ + _value: false, + __v_isRef: true, + }), + }); + + expect(fetchDocById).not.toHaveBeenCalled(); + + enableQuery.value = true; + await nextTick(); + + expect(fetchDocById).toHaveBeenCalledWith('users', `${mockUserId}/assignments/${mockAdministrationId}`); + }); + + it('should only fetch data if the params are set', async () => { + const mockUserId = ref(null); + const mockAdministrationId = ref(null); + + const queryOptions = { enabled: true }; + + withSetup(() => useUserAdministrationAssignmentsQuery(mockUserId, mockAdministrationId, queryOptions), { + plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], + }); + + expect(VueQuery.useQuery).toHaveBeenCalledWith({ + queryKey: ['user-administration-assignments', mockUserId, mockAdministrationId], + queryFn: expect.any(Function), + enabled: expect.objectContaining({ + _value: false, + __v_isRef: true, + }), + }); + + expect(fetchDocById).not.toHaveBeenCalled(); + + mockUserId.value = nanoid(); + + await nextTick(); + + expect(fetchDocById).not.toHaveBeenCalled(); + + mockAdministrationId.value = nanoid(); + + await nextTick(); + + expect(fetchDocById).toHaveBeenCalledWith('users', `${mockUserId}/assignments/${mockAdministrationId}`); + }); + + it('should not let queryOptions override the internally computed value', async () => { + const mockUserId = ref(null); + const mockAdministrationId = ref(nanoid()); + + const queryOptions = { enabled: true }; + + withSetup(() => useUserAdministrationAssignmentsQuery(mockUserId, mockAdministrationId, queryOptions), { + plugins: [[VueQuery.VueQueryPlugin, { queryClient }]], + }); + + expect(VueQuery.useQuery).toHaveBeenCalledWith({ + queryKey: ['user-administration-assignments', mockUserId, mockAdministrationId], + queryFn: expect.any(Function), + enabled: expect.objectContaining({ + _value: false, + __v_isRef: true, + }), + }); + + expect(fetchDocById).not.toHaveBeenCalled(); + }); +}); From 99068b6ec94cdcdb1ed82587f24968bed81c0400 Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 17:08:13 +0100 Subject: [PATCH 7/9] Fix linter offenses --- .../useUserAdministrationAssignmentsQuery.test.js | 6 +----- src/pages/StudentReport.vue | 9 ++------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js b/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js index 19f23d232..070928807 100644 --- a/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js +++ b/src/composables/queries/useUserAdministrationAssignmentsQuery.test.js @@ -1,10 +1,8 @@ import { ref, nextTick } from 'vue'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { createTestingPinia } from '@pinia/testing'; import * as VueQuery from '@tanstack/vue-query'; import { nanoid } from 'nanoid'; import { withSetup } from '@/test-support/withSetup.js'; -import { useAuthStore } from '@/store/auth'; import { fetchDocById } from '@/helpers/query/utils'; import useUserAdministrationAssignmentsQuery from './useUserAdministrationAssignmentsQuery'; @@ -21,11 +19,9 @@ vi.mock('@tanstack/vue-query', async (getModule) => { }); describe('useUserAdministrationAssignmentsQuery', () => { - let piniaInstance; let queryClient; beforeEach(() => { - piniaInstance = createTestingPinia(); queryClient = new VueQuery.QueryClient(); }); @@ -118,7 +114,7 @@ describe('useUserAdministrationAssignmentsQuery', () => { await nextTick(); - expect(fetchDocById).toHaveBeenCalledWith('users', `${mockUserId}/assignments/${mockAdministrationId}`); + expect(fetchDocById).toHaveBeenCalledWith('users', `${mockUserId.value}/assignments/${mockAdministrationId.value}`); }); it('should not let queryOptions override the internally computed value', async () => { diff --git a/src/pages/StudentReport.vue b/src/pages/StudentReport.vue index 681452ddb..1d30e9c89 100644 --- a/src/pages/StudentReport.vue +++ b/src/pages/StudentReport.vue @@ -181,7 +181,6 @@ import _startCase from 'lodash/startCase'; import { getGrade } from '@bdelab/roar-utils'; import { useAuthStore } from '@/store/auth'; import useUserDataQuery from '@/composables/queries/useUserDataQuery'; -import useUserAdministrationAssignmentsQuery from '@/composables/queries/useUserAdministrationAssignmentsQuery'; import useAdministrationsQuery from '@/composables/queries/useAdministrationsQuery'; import useUserRunPageQuery from '@/composables/queries/useUserRunPageQuery'; import { taskDisplayNames, addElementToPdf } from '@/helpers/reports'; @@ -218,19 +217,15 @@ const { data: studentData } = useUserDataQuery(props.userId, { enabled: initialized, }); -const { data: assignmentData } = useUserAdministrationAssignmentsQuery(props.userId, props.administrationId, { +const { data: administrationData } = useAdministrationsQuery([props.administrationId], { enabled: initialized, + select: (data) => data[0], }); const { data: taskData } = useUserRunPageQuery(props.userId, props.administrationId, props.orgType, props.orgId, { enabled: initialized, }); -const { data: administrationData } = useAdministrationsQuery([props.administrationId], { - enabled: initialized, - select: (data) => data[0], -}); - const expanded = ref(false); const exportLoading = ref(false); From 6ca311d972f059da2a6bbcf8a3ea849be6cb9f18 Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 17:10:27 +0100 Subject: [PATCH 8/9] Fix comment typos --- src/composables/queries/useUserRunPageQuery.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/composables/queries/useUserRunPageQuery.js b/src/composables/queries/useUserRunPageQuery.js index 5bc9c2766..f264c5b45 100644 --- a/src/composables/queries/useUserRunPageQuery.js +++ b/src/composables/queries/useUserRunPageQuery.js @@ -9,8 +9,9 @@ import { USER_RUN_PAGE_QUERY_KEY } from '@/constants/queryKeys'; /** * User run page query * - * @TODO: Evaluate whether this query can be replaced using more generic querie that already fetch user assessments and - * scores. This query was implemented as part of the transition to query composables but might be redudant. + * @TODO: Evaluate whether this query can be replaced using more generic query that already fetches user assessments and + * scores. This query was implemented as part of the transition to query composables but might be redudant if we + * refactor the underlying database query helpers to fetch all necessary data in a single query. * * @param {string|undefined|null} userId – The user ID to fetch, set to a falsy value to fetch the current user. * @param {QueryOptions|undefined} queryOptions – Optional TanStack query options. From f64e4dfe2f1d1f85ff84e1f8b9b077df100af00b Mon Sep 17 00:00:00 2001 From: Maximilian Oertel Date: Tue, 17 Sep 2024 17:30:10 +0100 Subject: [PATCH 9/9] Fix administration assignments query --- .../queries/useUserAdministrationAssignmentsQuery.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/composables/queries/useUserAdministrationAssignmentsQuery.js b/src/composables/queries/useUserAdministrationAssignmentsQuery.js index 3bd9776e5..df22ca430 100644 --- a/src/composables/queries/useUserAdministrationAssignmentsQuery.js +++ b/src/composables/queries/useUserAdministrationAssignmentsQuery.js @@ -19,7 +19,8 @@ const useUserAdministrationAssignmentsQuery = (userId, administrationId, queryOp return useQuery({ queryKey: [USER_ADMINISTRATION_ASSIGNMENTS_QUERY_KEY, userId, administrationId], - queryFn: () => fetchDocById(FIRESTORE_COLLECTIONS.USERS, `${userId}/assignments/${administrationId}`), + queryFn: () => + fetchDocById(FIRESTORE_COLLECTIONS.USERS, `${toValue(userId)}/assignments/${toValue(administrationId)}`), enabled: isQueryEnabled, ...options, });