Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KUI-1467 CourseRound dropdown suffix solely based on funding type #413

Open
wants to merge 2 commits into
base: upl-main
Choose a base branch
from

Conversation

belanglos
Copy link
Contributor

@belanglos belanglos commented Mar 19, 2025

KUI-1467

The main change is in useRoundUtils.js where the old suffix creation (containing special and default cases) is replaced by the new specification.

Had a bit of a hard time navigating overthinking and "getting-it-done" ;-)
As far as I understand it, we're semantically actually mixing at least 3 things here. (1) Funding type, (2) round type and the (3) "text we want to show to the users". Possibly (2) and (3) actually are the same, but I'm not sure round type does not also refer to something else semantically.

  • In the end, I opted with moving VU (also confirmed by Susanne to still hold meaning) from the removed round_category to round_type_suffix and implementing the matching in the code.
  • round_type in getFilteredData (exposed in context) is not used anymore, so I removed it and related variables.

@belanglos belanglos force-pushed the issues/KUI-1467-courseRound-funding-suffix branch from 7706888 to 0a2781e Compare March 25, 2025 09:31
@belanglos belanglos force-pushed the issues/KUI-1467-courseRound-funding-suffix branch from 0a2781e to ffb9b84 Compare March 25, 2025 09:43
@belanglos belanglos marked this pull request as ready for review March 25, 2025 09:51
Copy link
Contributor

@allazis allazis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ⭐

Have tested locally, checked the mapping according to the Excel and looked through the code. Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants