fix(KUI-1585): fix bugs in course analysis by simplifying semester lo…

…gic. The main changes are: - faulty week logic for determining semester is removed and only now semester from Kopps is used. - only one semester can be selected, to avoid issues of added same course offering multiple time. - ST is removed, course offerings will be included in HT och VT depending on semester from Kopps (ST was caluclated from weeks before and is no actual semester)
KTH · Nov 14, 2024 · 42bd5b0 · 42bd5b0
1 parent 904dadb
commit 42bd5b0
Show file tree

Hide file tree

Showing 27 changed files with 350 additions and 892 deletions.
diff --git a/domain/statistics/seasons.js b/domain/statistics/seasons.js
@@ -1,16 +1,10 @@
 const i18n = require('../../i18n')
 
-// eslint-disable-next-line no-unused-vars
 const seasonConstants = {
-  SPRING_TERM_NUMBER: 1, // Minimum possible term number.
-  AUTUMN_TERM_NUMBER: 2, // Maximum possible term number.
-  SUMMER_TERM_NUMBER: 0,
+  SPRING_TERM_NUMBER: 1,
+  AUTUMN_TERM_NUMBER: 2,
 }
-const orderedSeasons = () => [
-  seasonConstants.AUTUMN_TERM_NUMBER,
-  seasonConstants.SPRING_TERM_NUMBER,
-  seasonConstants.SUMMER_TERM_NUMBER,
-]
+const orderedSeasons = () => [seasonConstants.AUTUMN_TERM_NUMBER, seasonConstants.SPRING_TERM_NUMBER]
 /**
  * @param {number} seasonNumber
  * @param {number} langIndex
@@ -21,8 +15,6 @@ function labelSeason(seasonNumber, langIndex) {
   const { statisticsLabels: labels } = i18n.messages[langIndex]
 
   switch (Number(seasonNumber)) {
-    case seasonConstants.SUMMER_TERM_NUMBER:
-      return labels.seasonSummer
     case seasonConstants.AUTUMN_TERM_NUMBER:
       return labels.seasonAutumn
     case seasonConstants.SPRING_TERM_NUMBER:

diff --git a/i18n/messages.en.js b/i18n/messages.en.js
@@ -311,14 +311,14 @@ module.exports = {
         documentType: 'Area',
         periods: 'Study period',
         school: 'School',
-        seasons: 'Semester',
+        semester: 'Semester',
         year: 'Year',
       },
       formShortIntro: {
         documentType: 'Select the area you would like to see statistics for  ',
         periods: 'Choose one or more option(s)',
         school: 'Select school',
-        seasons: 'Choose one or more option(s)',
+        semester: 'Choose one semester',
         year: 'Select year',
       },
       missingParameters: { text: extra => `You have to select a ${extra} to view statistics.` },

diff --git a/i18n/messages.se.js b/i18n/messages.se.js
@@ -284,14 +284,14 @@ module.exports = {
         documentType: 'Område',
         periods: 'Läsperiod',
         school: 'Skola',
-        seasons: 'Termin',
+        semester: 'Termin',
         year: 'År',
       },
       formShortIntro: {
         documentType: 'Välj det område du vill se statistik för',
         periods: 'Kryssa i ett eller flera val',
         school: 'Välj skola',
-        seasons: 'Kryssa i ett eller flera val',
+        semester: 'Välj en termin',
         year: 'Välj år',
       },
       missingParameters: {

diff --git a/public/js/app/components/statistics/AnalysesSummary.jsx b/public/js/app/components/statistics/AnalysesSummary.jsx
@@ -25,7 +25,7 @@ function addAllSchoolsData({ totalCourses, totalUniqPublishedAnalyses }) {
   return allSchools
 }
 
-function Captions({ school, year, seasons }) {
+function Captions({ school, year, semester }) {
   const {
     languageIndex,
     translation: {
@@ -41,7 +41,7 @@ function Captions({ school, year, seasons }) {
     return schoolName
   }
 
-  const seasonsStr = seasons.map(season => seasonLib.labelSeason(season, languageIndex)).join(', ')
+  const semesterStr = seasonLib.labelSeason(semester, languageIndex)
   return (
     <Row>
       <Col xs="4" style={{ flex: 'none', width: 'auto', paddingBottom: '20px' }}>
@@ -53,8 +53,8 @@ function Captions({ school, year, seasons }) {
         {`: ${year}`}
       </Col>
       <Col xs="4" style={{ flex: 'none', width: 'auto', paddingBottom: '20px' }}>
-        <label>{formSubHeaders.seasons}</label>
-        {`: ${seasonsStr}`}
+        <label>{formSubHeaders.semester}</label>
+        {`: ${semesterStr}`}
       </Col>
     </Row>
   )
@@ -63,12 +63,12 @@ function Captions({ school, year, seasons }) {
 function AnalysesNumbersTable({ statisticsResult }) {
   const { translation } = useLanguage()
   const { analysesNumbersTable } = translation.statisticsLabels.summaryLabels
-  const { combinedAnalysesPerSchool, year, seasons, school } = statisticsResult
+  const { combinedAnalysesPerSchool, year, semester, school } = statisticsResult
   const cellNames = ['totalCourses', 'totalUniqPublishedAnalyses']
 
   return (
     <>
-      <Captions school={school} year={year} seasons={seasons} />
+      <Captions school={school} year={year} semester={semester} />
 
       <TableSummary
         docsPerSchool={combinedAnalysesPerSchool}
@@ -83,26 +83,26 @@ function AnalysesNumbersTable({ statisticsResult }) {
 
 function AnalysesNumbersCharts({ statisticsResult }) {
   const chartNames = ['numberOfUniqAnalyses']
-  const { combinedAnalysesPerSchool: docsPerSchool, seasons, year, school } = statisticsResult
+  const { combinedAnalysesPerSchool: docsPerSchool, semester, year, school } = statisticsResult
   const { schools = {} } = docsPerSchool
   if (school === 'allSchools') {
     schools.allSchools = addAllSchoolsData(docsPerSchool)
   }
 
   return (
     <>
-      <Captions school={school} year={year} seasons={seasons} />
+      <Captions school={school} year={year} semester={semester} />
 
       <Charts chartNames={chartNames} schools={schools} />
     </>
   )
 }
 function AnalysesNumbersChartsYearAgo({ statisticsResult }) {
-  const { school, documentType, seasons, year } = statisticsResult
+  const { school, documentType, semester, year } = statisticsResult
 
   const oneYearAgo = Number(year) - 1
 
-  const state = useStatisticsAsync({ seasons, year: oneYearAgo, documentType, school }, 'once')
+  const state = useStatisticsAsync({ semester, year: oneYearAgo, documentType, school }, 'once')
 
   const { data: statisticsResultYearAgo, status: statisticsStatus, error = {} } = state || {}
 

diff --git a/public/js/app/components/statistics/RadioboxOption.jsx b/public/js/app/components/statistics/RadioboxOption.jsx
@@ -71,7 +71,7 @@ function RadioboxOption({ paramName, onChange }) {
 }
 
 RadioboxOption.propTypes = {
-  paramName: PropTypes.oneOf(['documentType', 'school']).isRequired,
+  paramName: PropTypes.oneOf(['documentType', 'school', 'semester']).isRequired,
   onChange: PropTypes.func.isRequired,
 }
 

diff --git a/public/js/app/components/statistics/StatisticsDataTable.jsx b/public/js/app/components/statistics/StatisticsDataTable.jsx
@@ -303,7 +303,7 @@ function StatisticsDataTable({ statisticsResult }) {
     (statisticsResult.offeringsWithAnalyses && statisticsResult.offeringsWithAnalyses.length === 0)
   )
     return <NoDataMessage labels={sortableTable} />
-  const { year, offeringsWithMemos, periods = [], seasons = [], offeringsWithAnalyses } = statisticsResult
+  const { year, offeringsWithMemos, periods = [], semester = '', offeringsWithAnalyses } = statisticsResult
   const isMemoPage = offeringsWithMemos && offeringsWithMemos.length > 0 ? true : false
   const isAnalysisPage = offeringsWithAnalyses && offeringsWithAnalyses.length > 0 ? true : false
 
@@ -404,7 +404,7 @@ function StatisticsDataTable({ statisticsResult }) {
                 fileName={
                   isMemoPage
                     ? `course-information-statistics-memos-${year}-periods-${periods.join('-')}`
-                    : `course-information-statistics-analyses-${year}-periods-${seasons.join('-')}`
+                    : `course-information-statistics-analyses-${year}-periods-${semester}`
                 }
               ></StatisticsExport>
             </Row>

diff --git a/public/js/app/components/statistics/StatisticsForm.jsx b/public/js/app/components/statistics/StatisticsForm.jsx
@@ -85,12 +85,15 @@ function StatisticsForm({ onSubmit, resultError }) {
           </Row>
           <Row key={`row-for-periods-or-seasons-choice`} className={`row-for-periods-or-seasons-choice`}>
             <Col>
-              {/* depends on type of document to dropdown */}
-              <CheckboxOption
-                paramName={studyLengthParamName(documentType)}
-                onChange={handleParamChange}
-                stateMode={stateMode}
-              />
+              {documentType === 'courseAnalysis' ? (
+                <RadioboxOption paramName={studyLengthParamName(documentType)} onChange={handleParamChange} />
+              ) : (
+                <CheckboxOption
+                  paramName={studyLengthParamName(documentType)}
+                  onChange={handleParamChange}
+                  stateMode={stateMode}
+                />
+              )}
             </Col>
           </Row>
         </>

diff --git a/public/js/app/components/statistics/StatisticsResults.jsx b/public/js/app/components/statistics/StatisticsResults.jsx
@@ -31,9 +31,7 @@ SortableCoursesAndDocuments.propTypes = {
   languageIndex: PropTypes.oneOf([0, 1]),
   statisticsStatus: PropTypes.oneOf([...Object.values(STATUS), null]),
   statisticsResult: PropTypes.shape({
-    documentsApiBasePath: PropTypes.string,
     documentType: PropTypes.oneOf(documentTypes()),
-    koppsApiBasePath: PropTypes.string,
   }),
   error: PropTypes.shape({
     errorType: PropTypes.oneOf([...Object.values(ERROR_ASYNC), '']),

diff --git a/public/js/app/components/statistics/__tests__/StatisticsDataTable.test.js b/public/js/app/components/statistics/__tests__/StatisticsDataTable.test.js
@@ -38,7 +38,7 @@ const statisticsResultForMemo = {
 
 const statisticsResultForAnalysis = {
   year: 2019,
-  seasons: ['1'],
+  semester: '1',
   offeringsWithAnalyses: [
     {
       endDate: '2020-01-14',

diff --git a/public/js/app/components/statistics/__tests__/StatisticsFormEN.test.js b/public/js/app/components/statistics/__tests__/StatisticsFormEN.test.js
@@ -22,6 +22,7 @@ describe('Component <StatisticsForm> in english', () => {
   beforeAll(() => {
     jest.useFakeTimers({ advanceTimers: true })
     jest.setSystemTime(new Date(2023, 3, 1))
+    submittedResults = undefined
   })
 
   afterAll(() => {
@@ -267,21 +268,19 @@ describe('Component <StatisticsForm> in english', () => {
     expect(screen.getByRole('heading', { name: /semester/i })).toBeInTheDocument()
 
     await userEvent.click(screen.getByLabelText(/autumn/i))
-    await userEvent.click(screen.getByLabelText(/summer/i))
-
     expect(screen.getByLabelText(/autumn/i)).toBeChecked()
-    expect(screen.getByLabelText(/summer/i)).toBeChecked()
+
+    await userEvent.click(screen.getByLabelText(/spring/i))
+    expect(screen.getByLabelText(/spring/i)).toBeChecked()
+    expect(screen.getByLabelText(/autumn/i)).not.toBeChecked()
 
     await userEvent.click(btn)
     expect(submittedResults).toMatchInlineSnapshot(`
       {
         "documentType": "courseAnalysis",
         "periods": [],
         "school": undefined,
-        "seasons": [
-          2,
-          0,
-        ],
+        "semester": "1",
         "year": undefined,
       }
     `)
@@ -296,15 +295,15 @@ describe('Component <StatisticsForm> in english', () => {
     expect(courseAnalysis).toBeChecked()
 
     expect(screen.getByLabelText(/autumn/i)).not.toBeChecked()
-    expect(screen.getByLabelText(/summer/i)).not.toBeChecked()
+    expect(screen.getByLabelText(/spring/i)).not.toBeChecked()
 
     await userEvent.click(btn)
     expect(submittedResults).toMatchInlineSnapshot(`
       {
         "documentType": "courseAnalysis",
         "periods": [],
         "school": undefined,
-        "seasons": [],
+        "semester": undefined,
         "year": undefined,
       }
     `)
@@ -349,10 +348,11 @@ describe('Component <StatisticsForm> in english', () => {
     expect(screen.getByRole('heading', { name: /semester/i })).toBeInTheDocument()
 
     await userEvent.click(screen.getByLabelText(/autumn/i))
-    await userEvent.click(screen.getByLabelText(/summer/i))
-
     expect(screen.getByLabelText(/autumn/i)).toBeChecked()
-    expect(screen.getByLabelText(/summer/i)).toBeChecked()
+
+    await userEvent.click(screen.getByLabelText(/spring/i))
+    expect(screen.getByLabelText(/spring/i)).toBeChecked()
+    expect(screen.getByLabelText(/autumn/i)).not.toBeChecked()
 
     await userEvent.selectOptions(screen.getByRole('combobox', { name: /Select year/i }), '2019')
 
@@ -362,10 +362,7 @@ describe('Component <StatisticsForm> in english', () => {
         "documentType": "courseAnalysis",
         "periods": [],
         "school": "SCI",
-        "seasons": [
-          2,
-          0,
-        ],
+        "semester": "1",
         "year": 2019,
       }
     `)
@@ -402,7 +399,7 @@ describe('Component <StatisticsForm> in english', () => {
         "documentType": "courseAnalysis",
         "periods": [],
         "school": undefined,
-        "seasons": [],
+        "semester": undefined,
         "year": undefined,
       }
     `)
@@ -546,10 +543,8 @@ describe('Component <StatisticsForm> in english', () => {
     await userEvent.selectOptions(screen.getByRole('combobox', { name: /Select year/i }), '2019')
 
     await userEvent.click(screen.getByLabelText(/spring/i))
-    await userEvent.click(screen.getByLabelText(/summer/i))
-
     expect(screen.getByLabelText(/spring/i)).toBeChecked()
-    expect(screen.getByLabelText(/summer/i)).toBeChecked()
+    expect(screen.getByLabelText(/autumn/i)).not.toBeChecked()
 
     const btn = screen.getByRole('button', { name: /show statistics/i })
     await userEvent.click(btn)
@@ -558,10 +553,7 @@ describe('Component <StatisticsForm> in english', () => {
       {
         "documentType": "courseAnalysis",
         "school": "EECS",
-        "seasons": [
-          1,
-          0,
-        ],
+        "semester": "1",
         "year": 2019,
       }
     `)
@@ -574,18 +566,15 @@ describe('Component <StatisticsForm> in english', () => {
 
     await userEvent.click(screen.getByLabelText(/autumn/i))
     expect(screen.getByLabelText(/autumn/i)).toBeChecked()
+    expect(screen.getByLabelText(/spring/i)).not.toBeChecked()
 
     await userEvent.click(btn)
 
     expect(submittedResults).toMatchInlineSnapshot(`
       {
         "documentType": "courseAnalysis",
         "school": "ITM",
-        "seasons": [
-          1,
-          0,
-          2,
-        ],
+        "semester": "2",
         "year": 2020,
       }
     `)