Skip to content

Commit 3c17afd

Browse files
committed
fix(filters): scope drink facets by all filters but their own
Category, style, and allergen facets each used a different, ad-hoc scoping rule, so selecting one filter didn't consistently narrow the others and could hide the very option a user had just selected. Replace all three with one rule, implemented once in _scopeFor(): a facet is derived from every structural filter except its own. A selected option is always kept in its facet list/count even when its scoped count drops to 0, so an active filter (especially an allergen exclusion, a safety control) never disappears from the UI. Also fix availableAllergens to only list allergens with a non-zero value in scope, since a value of 0 means "confirmed absent" — previously zeroed keys inflated the allergen list with permanent no-op toggles. Free-text search stays excluded from facet scoping, unchanged. Fixes #317
1 parent b4cb99b commit 3c17afd

2 files changed

Lines changed: 490 additions & 35 deletions

File tree

lib/domain/controllers/drink_filter_controller.dart

Lines changed: 91 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,36 @@ import '../services/services.dart';
1414
///
1515
/// All mutators are synchronous and side-effect free; callers are responsible
1616
/// for persisting and broadcasting changes.
17+
///
18+
/// ## Facet scoping rule
19+
///
20+
/// [availableCategories], [categoryCountsMap], [availableStyles],
21+
/// [styleCountsMap], and [availableAllergens] are all derived by one rule:
22+
/// **a facet is computed from the source with every *other* structural
23+
/// filter applied — but never its own.** Structural filters are category,
24+
/// styles, favourites-only, visibility filters, and excluded allergens. A
25+
/// facet must not narrow itself, or selecting one of its own options would
26+
/// hide its siblings and the list would collapse under the user's finger
27+
/// (e.g. picking one style must not make every other style disappear from
28+
/// the style picker). See [_scopeFor], the single helper all five getters
29+
/// share.
30+
///
31+
/// Two invariants hold for every facet:
32+
/// 1. A currently-selected option is always listed, even when its scoped
33+
/// count is 0 — an active filter (especially an allergen exclusion,
34+
/// which is a safety filter) must never vanish from the UI the user
35+
/// would use to clear it.
36+
/// 2. [availableAllergens] lists only allergens actually present (a
37+
/// non-zero value) in scope, not merely mentioned with a value of 0 in a
38+
/// drink's allergens map — matching
39+
/// [DrinkFilterService.filterByExcludedAllergens]'s own definition of
40+
/// "absent".
41+
///
42+
/// Free-text search is deliberately **excluded** from facet scoping.
43+
/// `drinks_screen.dart` derives `hasStyleFilter` from
44+
/// `provider.availableStyles.isNotEmpty` to decide whether to show the Style
45+
/// button in the filter bar; scoping facets by the search query would make
46+
/// that button appear and disappear as the user types.
1747
class DrinkFilterController {
1848
final DrinkFilterService _filterService;
1949
final DrinkSortService _sortService;
@@ -55,52 +85,75 @@ class DrinkFilterController {
5585
/// Drinks after the active filters and sort have been applied.
5686
List<Drink> get filteredDrinks => _filtered;
5787

58-
/// Unique categories present in the source drinks, sorted.
88+
/// Unique categories present in scope (see class doc), sorted naturally.
89+
/// The [selectedCategory], if any, is always included even if its scoped
90+
/// count is 0 (invariant 1).
5991
List<String> get availableCategories {
60-
return _source.map((d) => d.category).toSet().toList()..sort();
92+
final categories = _scopeFor(
93+
_Facet.category,
94+
).map((d) => d.category).toSet();
95+
if (_selectedCategory != null) categories.add(_selectedCategory!);
96+
return categories.toList()..sort();
6197
}
6298

63-
/// Unique styles in the source drinks, narrowed to the selected category when
64-
/// one is active, sorted case-insensitively (via
65-
/// [StringComparisonHelper.compareCaseInsensitive]) so styles order in a stable,
66-
/// human-friendly way regardless of capitalisation. Presentation consumes
67-
/// this directly — no sorting in the UI.
99+
/// Unique styles in scope (see class doc), sorted case-insensitively (via
100+
/// [StringComparisonHelper.compareCaseInsensitive]) so styles order in a
101+
/// stable, human-friendly way regardless of capitalisation. Every
102+
/// [selectedStyles] entry is always included even if its scoped count is 0
103+
/// (invariant 1). Presentation consumes this directly — no sorting in the
104+
/// UI.
68105
List<String> get availableStyles {
69-
return _categoryScopedSource()
70-
.where((d) => d.style != null && d.style!.isNotEmpty)
71-
.map((d) => d.style!)
72-
.toSet()
73-
.toList()
74-
..sort(StringComparisonHelper.compareCaseInsensitive);
106+
final styles =
107+
_scopeFor(_Facet.style)
108+
.where((d) => d.style != null && d.style!.isNotEmpty)
109+
.map((d) => d.style!)
110+
.toSet()
111+
..addAll(_selectedStyles);
112+
return styles.toList()..sort(StringComparisonHelper.compareCaseInsensitive);
75113
}
76114

77-
/// Drink count per category across the full source.
115+
/// Drink count per category, scoped per the class doc. A [selectedCategory]
116+
/// with no matches in scope is still present, mapped to 0 (invariant 1).
78117
Map<String, int> get categoryCountsMap {
79118
final counts = <String, int>{};
80-
for (final drink in _source) {
119+
for (final drink in _scopeFor(_Facet.category)) {
81120
counts[drink.category] = (counts[drink.category] ?? 0) + 1;
82121
}
122+
if (_selectedCategory != null) {
123+
counts.putIfAbsent(_selectedCategory!, () => 0);
124+
}
83125
return counts;
84126
}
85127

86-
/// Drink count per style, narrowed to the selected category when one is
87-
/// active.
128+
/// Drink count per style, scoped per the class doc. Every entry of
129+
/// [selectedStyles] with no matches in scope is still present, mapped to 0
130+
/// (invariant 1).
88131
Map<String, int> get styleCountsMap {
89132
final counts = <String, int>{};
90-
for (final drink in _categoryScopedSource()) {
133+
for (final drink in _scopeFor(_Facet.style)) {
91134
if (drink.style != null && drink.style!.isNotEmpty) {
92135
counts[drink.style!] = (counts[drink.style!] ?? 0) + 1;
93136
}
94137
}
138+
for (final style in _selectedStyles) {
139+
counts.putIfAbsent(style, () => 0);
140+
}
95141
return counts;
96142
}
97143

98-
/// Every allergen key present across the source drinks.
144+
/// Allergens actually present (non-zero) on at least one drink in scope
145+
/// (see class doc and invariant 2), plus every currently
146+
/// [excludedAllergens] entry even if nothing in scope carries it
147+
/// (invariant 1) — a ticked allergen exclusion is a safety filter and must
148+
/// stay visible so the user can untick it.
99149
Set<String> get availableAllergens {
100150
final allergens = <String>{};
101-
for (final drink in _source) {
102-
allergens.addAll(drink.allergens.keys);
151+
for (final drink in _scopeFor(_Facet.allergen)) {
152+
for (final entry in drink.allergens.entries) {
153+
if (entry.value != 0) allergens.add(entry.key);
154+
}
103155
}
156+
allergens.addAll(_excludedAllergens);
104157
return allergens;
105158
}
106159

@@ -232,10 +285,21 @@ class DrinkFilterController {
232285
}
233286
}
234287

235-
/// Source narrowed to the selected category, or the full source when no
236-
/// category is selected.
237-
Iterable<Drink> _categoryScopedSource() {
238-
if (_selectedCategory == null) return _source;
239-
return _source.where((d) => d.category == _selectedCategory);
240-
}
288+
/// Source filtered by every structural criterion *except* the one
289+
/// belonging to [facet] — the single implementation of the facet-scoping
290+
/// rule documented on the class. Free-text search is intentionally never
291+
/// applied here (see class doc).
292+
Iterable<Drink> _scopeFor(_Facet facet) => _filterService.filterDrinks(
293+
_source,
294+
category: facet == _Facet.category ? null : _selectedCategory,
295+
styles: facet == _Facet.style ? const {} : _selectedStyles,
296+
favoritesOnly: _showFavoritesOnly,
297+
visibilityFilters: _visibilityFilters,
298+
excludedAllergens: facet == _Facet.allergen ? const {} : _excludedAllergens,
299+
// searchQuery intentionally omitted — see class doc "Facet scoping rule".
300+
);
241301
}
302+
303+
/// Which structural criterion a facet getter must not apply to itself. See
304+
/// [DrinkFilterController._scopeFor].
305+
enum _Facet { category, style, allergen }

0 commit comments

Comments
 (0)