Fix channels admin filters#5742
Fix channels admin filters#5742AlexVelezLl wants to merge 3 commits intolearningequality:unstablefrom
Conversation
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Clean implementation that meets both linked issues' acceptance criteria.
CI: only the spreadsheet workflow has run (passed); other checks still pending. Video demo confirms the filters work as expected — "All Channels" is the default, channel type is non-clearable, status filters are always enabled, and deleted channels are properly filtered.
- praise: Good introduction of
ChannelTypeFilterenum and use ofCommunityLibraryStatusconstants — eliminates magic strings and makes the filter logic self-documenting. - praise: The
defaultValueaddition touseFilteris a clean, minimal extension of the composable. - praise: Adding
{ immediate: true }to thechannelTypeFilterwatcher ensures the status filter is correctly initialized on first load.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Reviewed the pull request diff checking for:
- Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
- Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
- Architecture: duplicated concerns, minimal interfaces, composition over inheritance
- Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
- Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
- Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
- Checked CI status and linked issue acceptance criteria
- For UI changes: inspected screenshots for layout, visual completeness, and consistency
| import { CommunityLibraryStatus } from 'shared/constants'; | ||
|
|
||
| const ChannelTypeFilter = { | ||
| ALL: 'all', |
There was a problem hiding this comment.
praise: Good use of a ChannelTypeFilter enum to replace the previous magic strings ('kolibriStudio', 'community', 'unlisted'). This makes the filter logic easier to follow and less error-prone.
| const routeFilter = route.query[name]; | ||
| const filterOption = options.value.find(option => option.value === routeFilter); | ||
| return filterOption || {}; | ||
| return filterOption || options.value.find(option => option.value === defaultValue) || {}; |
There was a problem hiding this comment.
praise: Clean fallback chain — try the route query param first, then the default value, then empty object. Minimal change that adds defaultValue support without disrupting existing callers.
| channelStatusFilter.value = options.length ? options[0].value : null; | ||
| }, | ||
| { immediate: true }, | ||
| ); |
There was a problem hiding this comment.
praise: Adding { immediate: true } here is important — without it, the status filter wouldn't be initialized on first render when channelTypeFilter defaults to ALL. Good catch.
AlexVelezLl
force-pushed
the
fix-admin-filters
branch
from
86b4362 to
22478ad
Compare
3 participants
Summary
Grabacion.de.pantalla.2026-03-04.a.la.s.4.12.09.p.m.mov
References
Closes #5580.
Closes #5581.
Reviewer guidance