KP-433: add KTHPeriods to course search filter #371

belanglos · 2025-11-04T09:21:35Z

📌 Summary

This has to be merged after #370

Adds possibility to filter by KTH Periods to course search.

When visiting the page in spring, this years KTHPeriods and next years spring KTHPeriods should be available as filters.
When visiting the page in autumn, this years Autumn KTHPeriods and next years KTHPeriods should be available as filters.

Fixes https://kth-se.atlassian.net/browse/KP-433

🔍 Changes

updated code to use updated Search.searchCourseInstances endpoint from @kth/om-kursen-ladok-client
extracted functions into /shared to get TS help
added testing-library/user-event which is the preferred way to test UIs (as opposed to fireEvent)
removed deprecated defaultProps
renames ListView to StandardView in code
fixes layout in Search Table/Compact view
iterated on build and DX after ts-shared
minor typing improvements

✅ Checklist (Author)

Code builds locally without errors
Tests added/updated and passing
Linting/formatting applied
No sensitive data in the diff
No stray console.logs in the diff
No stray comments in the diff
Docs/README/CHANGELOG updated (if needed) (not applicable)
Sufficient logging
Errors are handled accordingly

🧪 Testing & Verification

Added unit tests for SearchFilters, which test which filters are displayed in spring and autumn. It also tests that clicking the filters calls the expected functions
Tested locally against new updated om-kursen-ladok-client

⚠️ Impact / Risks

🚧 Out of Scope

We could benefit from integration tests at this level.
Also, we do not have working cypress-E2E-tests for course search.

Copilot Summary

This pull request introduces several improvements and refactorings across the codebase, focusing on simplifying prop handling in React components, updating search parameter logic, enhancing internationalization, and improving development tooling. The most significant changes are grouped below.

Refactoring and Simplification of React Component Props

Removed unnecessary defaultProps assignments for Alert, Article, KoppsData, and LadokData components, instead setting default values directly in function parameters. This streamlines component definitions and reduces boilerplate. [1] [2] [3] [4] [5] [6] [7] [8]
Updated SearchDepartmentsProps to use the correct type for departmentCode, improving type safety.

Search Parameter Logic and Test Improvements

Refactored domain/searchParams.js to simplify period handling, remove unused functions, and utilize shared utilities for period configuration. This makes the search logic more maintainable and consistent. [1] [2]
Updated SearchFilters.test.tsx to use the correct types, improve test clarity, and switch to user-event for more realistic user interaction simulation. [1] [2] [3] [4] [5]

Internationalization Updates

Changed toggle button labels from list to standard in both English and Swedish message files to ensure consistency in UI terminology. [1] [2]

Development Tooling Enhancements

Updated the dev:shared script to clean the build directory before compiling TypeScript, preventing stale artifacts.
Added @testing-library/user-event as a dependency to support improved testing practices.

Cleanup

Removed completed and outdated items from the SearchTodos.MD file, keeping documentation current.

Copilot

Pull Request Overview

This PR refactors the course search functionality to improve period/semester handling by:

Replacing semester-based search parameters (semesters: ['HT2024', 'VT2024']) with KTH period-based parameters (period: ['2024:P1', '2024:summer'])
Moving shared search logic from JavaScript to TypeScript in the shared/ directory
Consolidating type definitions and removing duplicated code
Updating the webpack configuration to disable TypeScript project references to prevent build errors
Modernizing React components by replacing defaultProps with default parameter values

Reviewed Changes

Copilot reviewed 57 out of 61 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
webpack.config.js	Disables TypeScript project references in webpack to prevent TS6307 errors
tsconfig.json	Adds project references configuration for `shared` and `public` directories
shared/searchPageCtrlUtils.ts	New file containing TypeScript implementation of course search logic
shared/periodSearchUtils.ts	Refactored period configuration logic with new `convertSummerToPeriods` function
shared/periods.ts	Removes unused `getSummerPeriodsList` function
shared/languageUtil.ts	New shared language utility functions and types
shared/SearchTypes.ts	New shared TypeScript types for search responses
server/controllers/searchPageCtrl.js	Delegates to TypeScript implementation in shared directory
public/js/app/util/searchHelper.ts	Updates period display logic to use new KTH period data structures
public/js/app/pages/types/searchPageTypes.ts	Replaces `semesters` with `period` parameter and converts `SEARCH_MODES` to enum
domain/searchParams.js	Removes period transformation logic, delegates to shared TypeScript implementation
Various test files	Updates tests to use new period format (`2024:P1` instead of `HT2024`)
Various component files	Replaces `defaultProps` with default parameter values per React best practices

Comments suppressed due to low confidence (3)

shared/periodSearchUtils.ts:1

Corrected spelling of 'periodSearchutils' to 'periodSearchUtils' to match the file name.
domain/searchParams.js:136
Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('currentYear', langIndex, true)

domain/searchParams.js:138

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('nextYear', langIndex, true)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/__tests__/periodSearchUtils.test.ts

public/js/app/stores/types/searchPageStoreTypes.ts

public/js/app/components/SearchFilters/SearchFilters.test.tsx

public/js/app/components/Article.jsx

public/js/app/components/SearchFilters/SearchFilters.test.tsx

shared/searchPageCtrlUtils.ts

Copilot · 2025-11-04T09:38:23Z

@belanglos I've opened a new pull request, #372, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull Request Overview

Copilot reviewed 57 out of 61 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

public/js/app/pages/types/searchPageTypes.ts:1

The Period type definition is incorrect. It only allows 'P' followed by '1' | '2' | '3' | '4' | 'summer', but the actual period values used throughout the code include 'P0' and 'P5' (e.g., in tests and convertSummerToPeriods function). The type should be ${number}:P${'0' | '1' | '2' | '3' | '4' | '5'} or ${number}:${'summer' | \P${0 | 1 | 2 | 3 | 4 | 5}`}`.
domain/searchParams.js:136
Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('currentYear', langIndex, true)

domain/searchParams.js:138

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('nextYear', langIndex, true)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/searchPageCtrlUtils.ts

public/js/app/components/SearchFilters/types.ts

Copilot

Pull Request Overview

Copilot reviewed 57 out of 61 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

domain/searchParams.js:136

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('currentYear', langIndex, true)

domain/searchParams.js:138

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('nextYear', langIndex, true)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/__tests__/periodSearchUtils.test.ts

public/js/app/util/searchHelper.ts

public/js/app/components/mocks/mockSearchData.ts

public/js/app/components/SearchFilters/index.tsx

public/js/app/components-shared/Alert.jsx

Copilot

Pull Request Overview

Copilot reviewed 57 out of 61 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

public/js/app/components-shared/Alert.jsx:1

The type prop is marked as isRequired in PropTypes but has a default value of 'info' in the function signature. Remove .isRequired to align with the default parameter behavior.

// https://intra.kth.se/style/en/components/alert

domain/searchParams.js:136

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('currentYear', langIndex, true)

domain/searchParams.js:138

Superfluous argument passed to function _periodConfigByYearType.

      return _periodConfigByYearType('nextYear', langIndex, true)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shared/periodSearchUtils.ts

Avoid TS project reference mode inside webpack build to prevent TS6307 We build referenced projects (e.g. shared/) separately via npm scripts and resolve them via path/webpack aliases during bundling.

…sages

axelbjo

So much work done here, really nice job! I like the refactoring and general restructuring of things throughout the repo. Nice to see more TS introduced too.

Tested the application locally and ran the tests as well, no problems there. Great job Benni!

belanglos requested a review from Copilot November 4, 2025 09:21

belanglos added 2 commits November 4, 2025 10:23

test(KP-433): fix third cycle TableView test

2ebf849

chore(KP-433): satisfy ESLint

277f61c

Copilot AI reviewed Nov 4, 2025

View reviewed changes

Copilot AI mentioned this pull request Nov 4, 2025

Clarify PropTypes comment - no changes needed #372

Closed

belanglos force-pushed the issues/KP-433-implement-periods-without-shared branch 2 times, most recently from c7ccca1 to 51442a4 Compare November 4, 2025 09:42

belanglos requested a review from Copilot November 4, 2025 09:43

belanglos changed the title ~~test(KP-430): fix third cycle TableView test~~ KP-433: add KTHPeriods to course search filter Nov 4, 2025

Copilot AI reviewed Nov 4, 2025

View reviewed changes

shared/searchPageCtrlUtils.ts Show resolved Hide resolved

public/js/app/components/SearchFilters/types.ts Outdated Show resolved Hide resolved

belanglos force-pushed the issues/KP-433-implement-periods-without-shared branch from 5a6e2b2 to 322e94d Compare November 4, 2025 10:47

belanglos requested a review from Copilot November 4, 2025 10:47

Copilot AI reviewed Nov 4, 2025

View reviewed changes

belanglos force-pushed the issues/KP-433-implement-periods-without-shared branch from efab51b to 6e9f229 Compare November 4, 2025 10:59

belanglos requested a review from Copilot November 4, 2025 10:59

Copilot AI reviewed Nov 4, 2025

View reviewed changes

shared/periodSearchUtils.ts Show resolved Hide resolved

belanglos added 13 commits November 4, 2025 12:10

build(KP-433): fix resolving issue when running npm run start-dev

aceaaf8

Avoid TS project reference mode inside webpack build to prevent TS6307 We build referenced projects (e.g. shared/) separately via npm scripts and resolve them via path/webpack aliases during bundling.

ref(KP-433): refactor periodSearchUtils

834ee1f

ref(KP-433): remove deprecated defaultProps to get rid of warning mes…

553c50e

…sages

ref(KP-433): break out performCourseSearch into TS to leverage types

bdfebf3

ref(KP-433): refactor Search result views

ccf5d82

test(KP-433): Programme.test.js contains react -> jsx

7a9020c

feat(KP-433) implement KTH period filters for search

68ba0e7

ref(KP-433): remove unused variable

dce6e38

chore(KP-433): remove unused comments

6dce09b

chore(KP-433): remove SearchTodos

bbadd45

ref(KP-433): rename ListView to StandardView

f01abea

fix(KP-433): do not wrap heading rows in Table/Compact layout

d0723e9

chore(KP-433): add tl/user-events for testing

2e8b5dc

test(KP-433): add unit tests for period filters

2e5762a

belanglos force-pushed the issues/KP-433-implement-periods-without-shared branch from 6e9f229 to 2e5762a Compare November 4, 2025 11:10

belanglos marked this pull request as ready for review November 4, 2025 11:14

belanglos requested review from allazis, amirhossein-haerian and axelbjo as code owners November 4, 2025 11:14

axelbjo approved these changes Nov 11, 2025

View reviewed changes

KP-433: add KTHPeriods to course search filter #371

Are you sure you want to change the base?

KP-433: add KTHPeriods to course search filter #371

Uh oh!

Conversation

belanglos commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Summary

🔍 Changes

✅ Checklist (Author)

🧪 Testing & Verification

⚠️ Impact / Risks

🚧 Out of Scope

Copilot Summary

Refactoring and Simplification of React Component Props

Search Parameter Logic and Test Improvements

Internationalization Updates

Development Tooling Enhancements

Cleanup

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Nov 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

axelbjo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

belanglos commented Nov 4, 2025 •

edited

Loading