fix(KUI-1176) bug in useSemesterRoundState

KTH · May 17, 2024 · a0cca90 · a0cca90
1 parent 66f2744
commit a0cca90
Show file tree

Hide file tree

Showing 5 changed files with 342 additions and 370 deletions.
diff --git a/...p/hooks/__tests__/useActiveRounds.test.js → ...s/__tests__/useSemesterRoundState.test.js b/...p/hooks/__tests__/useActiveRounds.test.js → ...s/__tests__/useSemesterRoundState.test.js
@@ -214,12 +214,12 @@ const activeSemesters = [
 ]
 
 const { renderHook, waitFor, act } = require('@testing-library/react')
-const { useActiveRounds } = require('../useActiveRounds')
+const { useSemesterRoundState } = require('../useSemesterRoundState')
 
 describe('useSemesterRoundsLogic', () => {
   test('returns correct values', () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20242,
         roundList,
@@ -231,7 +231,6 @@ describe('useSemesterRoundsLogic', () => {
     expect(result.current.selectedRoundIndex).toBe(undefined)
     expect(result.current.activeRound).toEqual({})
     expect(result.current.selectedSemester).toBe(20242)
-    expect(result.current.isSetSelectedRoundIndex).toBe(false)
     expect(result.current.showRoundData).toBe(false)
     expect(result.current.activeSemesterOnlyHasOneRound).toBe(false)
     expect(result.current.firstRoundInActiveSemester).toEqual(roundList[20242][0])
@@ -240,9 +239,9 @@ describe('useSemesterRoundsLogic', () => {
     expect(result.current.hasSyllabus).toBe(true)
   })
 
-  test('updates selectedRoundIndex, isSetSelectedRoundIndex, showRoundData and activeRound when setSelectedRoundIndex is called', async () => {
+  test('updates selectedRoundIndex, showRoundData and activeRound when setSelectedRoundIndex is called', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20242,
         roundList,
@@ -252,22 +251,20 @@ describe('useSemesterRoundsLogic', () => {
     )
     expect(result.current.selectedRoundIndex).toBe(undefined)
     expect(result.current.activeRound).toEqual({})
-    expect(result.current.isSetSelectedRoundIndex).toBe(false)
     expect(result.current.showRoundData).toBe(false)
 
     act(() => {
       result.current.setSelectedRoundIndex(0)
     })
 
     await waitFor(() => expect(result.current.selectedRoundIndex).toBe(0))
-    await waitFor(() => expect(result.current.isSetSelectedRoundIndex).toBe(true))
     await waitFor(() => expect(result.current.showRoundData).toBe(true))
     await waitFor(() => expect(result.current.activeRound).toEqual(roundList[20242][0]))
   })
 
   test('updates selectedSemester, firstRoundInActiveSemester, when setSelectedSemester is called', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20242,
         roundList,
@@ -284,9 +281,9 @@ describe('useSemesterRoundsLogic', () => {
     await waitFor(() => expect(result.current.firstRoundInActiveSemester).toBe(roundList[20232][0]))
   })
 
-  test('if selectedRoundIndex is set and setSelectedSemester is called, reset selectedRoundIndex, activeRound, isSetSelectedRoundIndex and showRoundData', async () => {
+  test('if selectedRoundIndex is set and setSelectedSemester is called, reset selectedRoundIndex, activeRound and showRoundData', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20242,
         roundList,
@@ -295,7 +292,6 @@ describe('useSemesterRoundsLogic', () => {
       })
     )
     expect(result.current.selectedRoundIndex).toBe(undefined)
-    expect(result.current.isSetSelectedRoundIndex).toBe(false)
     expect(result.current.showRoundData).toBe(false)
     expect(result.current.activeRound).toEqual({})
 
@@ -304,22 +300,20 @@ describe('useSemesterRoundsLogic', () => {
     })
 
     await waitFor(() => expect(result.current.selectedRoundIndex).toBe(0))
-    await waitFor(() => expect(result.current.isSetSelectedRoundIndex).toBe(true))
     await waitFor(() => expect(result.current.showRoundData).toBe(true))
     await waitFor(() => expect(result.current.activeRound).toEqual(roundList[20242][0]))
 
     act(() => {
       result.current.setSelectedSemester(20232)
     })
     await waitFor(() => expect(result.current.selectedRoundIndex).toBe(undefined))
-    await waitFor(() => expect(result.current.isSetSelectedRoundIndex).toBe(false))
     await waitFor(() => expect(result.current.showRoundData).toBe(false))
     await waitFor(() => expect(result.current.activeRound).toEqual({}))
   })
 
   test('if a semester with only one round is selected, that round should be selected', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20222,
         roundList,
@@ -330,14 +324,12 @@ describe('useSemesterRoundsLogic', () => {
     expect(result.current.activeSemesterOnlyHasOneRound).toBe(true)
     expect(result.current.showRoundData).toBe(true)
 
-    expect(result.current.selectedRoundIndex).toBe(0)
-    expect(result.current.isSetSelectedRoundIndex).toBe(true)
     expect(result.current.activeRound).toEqual(roundList[20222][0])
   })
 
   test('if initiallySelectedSemester is a string, converts it to number', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: '20222',
         roundList,
@@ -353,7 +345,7 @@ describe('useSemesterRoundsLogic', () => {
     'if roundList is empty or undefined, returns values accordingly',
     async invalidRoundList => {
       const { result } = renderHook(() =>
-        useActiveRounds({
+        useSemesterRoundState({
           initiallySelectedRoundIndex: undefined,
           initiallySelectedSemester: 20242,
           roundList: invalidRoundList,
@@ -367,14 +359,13 @@ describe('useSemesterRoundsLogic', () => {
       expect(result.current.firstRoundInActiveSemester).toEqual({})
 
       expect(result.current.selectedRoundIndex).toBe(undefined)
-      expect(result.current.isSetSelectedRoundIndex).toBe(false)
       expect(result.current.activeRound).toEqual({})
     }
   )
 
   test('picks correct syllabus for selected semester', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20182,
         roundList,
@@ -389,7 +380,7 @@ describe('useSemesterRoundsLogic', () => {
 
   test('if no syllabuses are, sets activeSyllabus to undefined and hasSyllabus to false', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20182,
         roundList,
@@ -404,7 +395,7 @@ describe('useSemesterRoundsLogic', () => {
 
   test('if no valid syllabus is given, sets activeSyllabus to undefined and hasSyllabus to false', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20182,
         roundList,
@@ -419,7 +410,7 @@ describe('useSemesterRoundsLogic', () => {
 
   test('calling resetSelectedRoundIndex resets round values', async () => {
     const { result } = renderHook(() =>
-      useActiveRounds({
+      useSemesterRoundState({
         initiallySelectedRoundIndex: undefined,
         initiallySelectedSemester: 20242,
         roundList,
@@ -429,7 +420,6 @@ describe('useSemesterRoundsLogic', () => {
     )
 
     expect(result.current.selectedRoundIndex).toBe(undefined)
-    expect(result.current.isSetSelectedRoundIndex).toBe(false)
     expect(result.current.showRoundData).toBe(false)
     expect(result.current.activeRound).toEqual({})
 
@@ -438,15 +428,13 @@ describe('useSemesterRoundsLogic', () => {
     })
 
     await waitFor(() => expect(result.current.selectedRoundIndex).toBe(0))
-    await waitFor(() => expect(result.current.isSetSelectedRoundIndex).toBe(true))
     await waitFor(() => expect(result.current.showRoundData).toBe(true))
     await waitFor(() => expect(result.current.activeRound).toEqual(roundList[20242][0]))
 
     act(() => {
       result.current.resetSelectedRoundIndex()
     })
     await waitFor(() => expect(result.current.selectedRoundIndex).toBe(undefined))
-    await waitFor(() => expect(result.current.isSetSelectedRoundIndex).toBe(false))
     await waitFor(() => expect(result.current.showRoundData).toBe(false))
     await waitFor(() => expect(result.current.activeRound).toEqual({}))
   })

diff --git a/public/js/app/hooks/useRoundUtils.js b/public/js/app/hooks/useRoundUtils.js
@@ -30,7 +30,7 @@ export const useRoundUtils = () => {
 
         return `${semesterStringOrEmpty}${roundYear} ${roundLabel}`
       },
-    [createRoundLabel]
+    [createRoundLabel, translation.courseInformation.course_short_semester]
   )
 
   return {

diff --git a/public/js/app/hooks/useActiveRounds.js → public/js/app/hooks/useSemesterRoundState.js b/public/js/app/hooks/useActiveRounds.js → public/js/app/hooks/useSemesterRoundState.js
@@ -1,16 +1,16 @@
-const { useState, useMemo, useCallback, useEffect } = require('react')
+const { useState, useMemo, useCallback } = require('react')
 const { getValidSyllabusForSemester } = require('./getValidSyllabusForSemester')
 
 const UNSET_VALUE = undefined
 
 const getElementOrEmpty = (arr, index) => {
-  if (!arr || arr.length === 0 || index === undefined) {
+  if (!arr || arr.length === 0 || index === undefined || index >= arr.length) {
     return {}
   }
   return arr[index]
 }
 
-const useActiveRounds = ({
+const useSemesterRoundState = ({
   initiallySelectedRoundIndex,
   initiallySelectedSemester,
   roundList,
@@ -22,22 +22,14 @@ const useActiveRounds = ({
 
   const isSetSelectedRoundIndex = useMemo(() => selectedRoundIndex !== UNSET_VALUE, [selectedRoundIndex])
 
-  const resetSelectedRoundIndex = useCallback(() => setSelectedRoundIndex(UNSET_VALUE), [setSelectedRoundIndex])
+  const resetSelectedRoundIndex = useCallback(() => setSelectedRoundIndex(() => UNSET_VALUE), [setSelectedRoundIndex])
 
   const semesterOnlyHasOneRound = (rounds, semester) =>
     rounds !== undefined &&
     Object.hasOwnProperty.call(rounds, semester) &&
     rounds[semester] &&
     rounds[semester].length === 1
 
-  useEffect(() => {
-    if (semesterOnlyHasOneRound(roundList, selectedSemester)) {
-      setSelectedRoundIndex(0)
-    } else {
-      resetSelectedRoundIndex()
-    }
-  }, [resetSelectedRoundIndex, roundList, selectedSemester])
-
   const activeSemesterOnlyHasOneRound = useMemo(
     () =>
       roundList !== undefined &&
@@ -68,10 +60,11 @@ const useActiveRounds = ({
     [roundsForActiveSemester]
   )
 
-  const activeRound = useMemo(
-    () => getElementOrEmpty(roundsForActiveSemester, selectedRoundIndex),
-    [roundsForActiveSemester, selectedRoundIndex]
-  )
+  const activeRound = useMemo(() => {
+    const index = activeSemesterOnlyHasOneRound ? 0 : selectedRoundIndex
+
+    return getElementOrEmpty(roundsForActiveSemester, index)
+  }, [activeSemesterOnlyHasOneRound, roundsForActiveSemester, selectedRoundIndex])
 
   const hasActiveSemesters = useMemo(() => activeSemesters && activeSemesters.length > 0, [activeSemesters])
 
@@ -85,15 +78,22 @@ const useActiveRounds = ({
     [activeSyllabus, syllabusList]
   )
 
-  const setSelectedSemesterAsNumber = useCallback(newActiveSemester => {
-    setSelectedSemester(Number(newActiveSemester))
-  }, [])
+  const setSelectedSemesterAsNumber = useCallback(
+    newActiveSemester => {
+      setSelectedSemester(() => Number(newActiveSemester))
+      if (semesterOnlyHasOneRound(roundList, selectedSemester)) {
+        setSelectedRoundIndex(0)
+      } else {
+        resetSelectedRoundIndex()
+      }
+    },
+    [resetSelectedRoundIndex, roundList, selectedSemester]
+  )
 
   return {
     selectedRoundIndex,
     activeRound,
     selectedSemester,
-    isSetSelectedRoundIndex,
     showRoundData,
     activeSemesterOnlyHasOneRound,
     firstRoundInActiveSemester,
@@ -107,5 +107,5 @@ const useActiveRounds = ({
 }
 
 module.exports = {
-  useActiveRounds,
+  useSemesterRoundState,
 }
diff --git a/public/js/app/pages/CoursePage.jsx b/public/js/app/pages/CoursePage.jsx
@@ -19,7 +19,7 @@ import { useWebContext } from '../context/WebContext'
 import BankIdAlert from '../components/BankIdAlert'
 import { useLanguage } from '../hooks/useLanguage'
 import { useMissingInfo } from '../hooks/useMissingInfo'
-import { useActiveRounds } from '../hooks/useActiveRounds'
+import { useSemesterRoundState } from '../hooks/useSemesterRoundState'
 import { convertYearSemesterNumberIntoSemester } from '../../../../server/util/semesterUtils'
 
 const aboutCourseStr = (translate, courseCode = '') => `${translate.site_name} ${courseCode}`
@@ -41,7 +41,7 @@ function CoursePage() {
   } = context
   // * * //
 
-  const semesterRoundState = useActiveRounds({
+  const semesterRoundState = useSemesterRoundState({
     initiallySelectedRoundIndex,
     initiallySelectedSemester,
     roundList: courseData.roundList,
@@ -50,7 +50,6 @@ function CoursePage() {
   })
   const {
     selectedSemester,
-    isSetSelectedRoundIndex,
     firstRoundInActiveSemester,
     activeRound,
     showRoundData,
@@ -150,7 +149,7 @@ function CoursePage() {
             <div className="paragraphs" dangerouslySetInnerHTML={{ __html: sellingText }} />
           </Col>
         </section>
-        {courseData.roundList && hasActiveSemesters && showRoundData && isSetSelectedRoundIndex && (
+        {showRoundData && (
           <BankIdAlert
             tutoringForm={activeRound.round_tutoring_form}
             fundingType={activeRound.round_funding_type}
@@ -214,7 +213,7 @@ function CoursePage() {
                   )}
 
                   {/* ---ROUND CANCELLED OR FULL --- */}
-                  {showRoundData && isSetSelectedRoundIndex && activeRound.round_state !== 'APPROVED' ? (
+                  {showRoundData && activeRound.round_state !== 'APPROVED' ? (
                     <Alert type="info" aria-live="polite">
                       {`${translation.courseLabels.lable_round_state[activeRound.round_state]}
                             `}
@@ -239,7 +238,7 @@ function CoursePage() {
             )}
 
             {/* ---COURSE ROUND INFORMATION--- */}
-            {hasActiveSemesters && isSetSelectedRoundIndex ? (
+            {showRoundData ? (
               <RoundInformationOneCol
                 memoStorageURI={browserConfig.memoStorageUri}
                 semesterRoundState={semesterRoundState}