Changed featuredPresentation to support multiple featured presentations#1019
Merged
SzBeni2003 merged 4 commits intostagingfrom Mar 4, 2026
Merged
Changed featuredPresentation to support multiple featured presentations#1019SzBeni2003 merged 4 commits intostagingfrom
SzBeni2003 merged 4 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR converts featured presentation support from a single item to multiple items: models, API fields, service fetch logic, and configuration now use lists and comma-separated selectors instead of single values. (28 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API\ Controller
participant Service as ConferenceService
participant Repo as PresentationRepo
participant DB as Database
rect rgba(135,206,235,0.5)
Client->>API: GET index page
API->>Service: buildConfEntities()
Service->>Service: read setting featuredPresentationSelectors
Service->>Repo: findTop1BySelector(selector1)
Repo->>DB: query presentation by selector1
DB-->>Repo: presentationEntity?
Repo-->>Service: presentationEntity (or null)
Service->>Repo: findTop1BySelector(selectorN)
Repo->>DB: query presentation by selectorN
DB-->>Repo: presentationEntity?
Repo-->>Service: presentationEntity (or null)
Service->>Service: fetchPresentation(entity) for non-null results
Service-->>API: ConfEntities(with featuredPresentations list)
API-->>Client: JSON index page with featuredPresentations
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceComponent.kt`:
- Line 109: The setting property was renamed from featuredPresentationSelector
to featuredPresentationSelectors so existing data keyed by the old property will
be lost; update storage mapping or migrate data by either adding a DB migration
that copies/renames the key from "featuredPresentationSelector" to
"featuredPresentationSelectors" or keep backward compatibility by supplying the
old key explicitly to the setting helper (e.g. use StringSettingRef with
fieldName set to the original key) so ComponentSettingService can still locate
existing values; modify the code around featuredPresentationSelectors and/or add
a migration to perform the rename in the database.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt`:
- Around line 32-37: The code is querying the repository for blank selector
tokens; before calling conferencePresentationRepository.findTop1BySelector and
fetchPresentation, split and trim the selector string
(conferenceComponent.featuredPresentationSelectors) and filter out empty/blank
tokens (e.g., use trim().isNotEmpty()) so you only perform lookups for real
selectors; update the pipeline around featuredPresentationSelectors ->
findTop1BySelector -> fetchPresentation to first map to trimmed tokens and
filter blanks before mapping to repository calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afcb0239-b7b5-4baf-a402-312916d6fdd3
📒 Files selected for processing (4)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceApiController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/IndexPageData.kt
| type = SettingType.LONG_TEXT, fieldName = "featuredPresentation.description mező") | ||
|
|
||
| var featuredPresentationSelector by StringSettingRef("presentation-1", fieldName = "Kiemelt előadás selectorja") | ||
| var featuredPresentationSelectors by StringSettingRef("", fieldName = "Kiemelt előadások selectorjai, vesszővel elválasztva") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find where StringSettingRef is defined
find . -type f -name "*.kt" | xargs grep -l "class StringSettingRef" | head -5Repository: kir-dev/cmsch
Length of output: 122
🏁 Script executed:
# Search for StringSettingRef implementation and key derivation
rg -n "class StringSettingRef|provideDelegate|getValue|fieldName" backend/src/main/kotlin --context 5 -A 10Repository: kir-dev/cmsch
Length of output: 50370
🏁 Script executed:
# Check both old and new property names in the file
rg -n "featuredPresentationSelector" backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceComponent.ktRepository: kir-dev/cmsch
Length of output: 190
🏁 Script executed:
# Look for migration-related code or database scripts
fd -e sql -e kt | xargs grep -l "migration\|Migration\|featured.*selector" 2>/dev/null | head -20Repository: kir-dev/cmsch
Length of output: 141
🏁 Script executed:
# Check if there's any backward compatibility or key mapping logic
rg -n "featuredPresentationSelector|oldKey|previousKey|compat" backend/src/main/kotlinRepository: kir-dev/cmsch
Length of output: 480
Add a database migration or setting key mapping for the property rename.
The property was renamed from singular to plural, changing the storage key from featuredPresentationSelector to featuredPresentationSelectors. Since ComponentSettingService uses the Kotlin property name as the database query key, existing configuration data will become unreachable. Either migrate the old key to the new one in the database, or provide explicit key mapping via the fieldName parameter to maintain backward compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceComponent.kt`
at line 109, The setting property was renamed from featuredPresentationSelector
to featuredPresentationSelectors so existing data keyed by the old property will
be lost; update storage mapping or migrate data by either adding a DB migration
that copies/renames the key from "featuredPresentationSelector" to
"featuredPresentationSelectors" or keep backward compatibility by supplying the
old key explicitly to the setting helper (e.g. use StringSettingRef with
fieldName set to the original key) so ComponentSettingService can still locate
existing values; modify the code around featuredPresentationSelectors and/or add
a migration to perform the rename in the database.
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt (1)
32-38: Remove redundant trim in selector pipeline.Line 33 already trims tokens, so Line 36 can use
presentationdirectly. This is minor, but it simplifies the pipeline.♻️ Proposed cleanup
val featuredPresentations = conferenceComponent.featuredPresentationSelectors.split(",") .map{ it.trim() }.filter { it.isNotBlank() } .mapNotNull { presentation -> conferencePresentationRepository - .findTop1BySelector(presentation.trim()) + .findTop1BySelector(presentation) .firstOrNull() ?.let { fetchPresentation(it) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt` around lines 32 - 38, The pipeline that builds featuredPresentations trims tokens at map{ it.trim() }, so remove the redundant .trim() call inside the mapNotNull block; update the use of presentation in conferencePresentationRepository.findTop1BySelector(...) to pass presentation directly and then continue to call fetchPresentation(...) as before (affects featuredPresentations, conferenceComponent.featuredPresentationSelectors, conferencePresentationRepository.findTop1BySelector and fetchPresentation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt`:
- Around line 32-38: The pipeline that builds featuredPresentations trims tokens
at map{ it.trim() }, so remove the redundant .trim() call inside the mapNotNull
block; update the use of presentation in
conferencePresentationRepository.findTop1BySelector(...) to pass presentation
directly and then continue to call fetchPresentation(...) as before (affects
featuredPresentations, conferenceComponent.featuredPresentationSelectors,
conferencePresentationRepository.findTop1BySelector and fetchPresentation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16edda4d-299b-4760-b720-4a622945871c
📒 Files selected for processing (1)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt (1)
32-38: Consider improving naming and formatting for readability.The lambda parameter
presentationis misleading since it's actually a selector string, not a presentation entity. Also, the indentation inside themapNotNullblock is inconsistent.✨ Suggested formatting and naming improvement
- val featuredPresentations = conferenceComponent.featuredPresentationSelectors.split(",") - .map{ it.trim() }.filter { it.isNotBlank() } - .mapNotNull { presentation -> - conferencePresentationRepository - .findTop1BySelector(presentation) - .firstOrNull() - ?.let { fetchPresentation(it) }} + val featuredPresentations = conferenceComponent.featuredPresentationSelectors + .split(",") + .map { it.trim() } + .filter { it.isNotBlank() } + .mapNotNull { selector -> + conferencePresentationRepository + .findTop1BySelector(selector) + .firstOrNull() + ?.let { fetchPresentation(it) } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt` around lines 32 - 38, Rename the misleading lambda parameter and tidy indentation: treat the input as a selector string (e.g., rename `presentation` to `selector`) in the chain that starts from `conferenceComponent.featuredPresentationSelectors.split(",")`, update the `mapNotNull` lambda accordingly to call `conferencePresentationRepository.findTop1BySelector(selector)` and then `fetchPresentation(it)`, and fix the block indentation so the `.mapNotNull { selector -> conferencePresentationRepository.findTop1BySelector(selector).firstOrNull()?.let { fetchPresentation(it) } }` (or equivalent multi-line formatting) is clean and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt`:
- Around line 32-38: Rename the misleading lambda parameter and tidy
indentation: treat the input as a selector string (e.g., rename `presentation`
to `selector`) in the chain that starts from
`conferenceComponent.featuredPresentationSelectors.split(",")`, update the
`mapNotNull` lambda accordingly to call
`conferencePresentationRepository.findTop1BySelector(selector)` and then
`fetchPresentation(it)`, and fix the block indentation so the `.mapNotNull {
selector ->
conferencePresentationRepository.findTop1BySelector(selector).firstOrNull()?.let
{ fetchPresentation(it) } }` (or equivalent multi-line formatting) is clean and
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e825b8a6-9a91-46a9-9565-7e9494711544
📒 Files selected for processing (1)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/conference/ConferenceService.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit