Conversation
Adds a new filter option to the Entity Collection View (ECV) that allows filtering based on items present in specified Organizer profiles. This allows users to easily find items that match their Organizer rules.
WalkthroughThe PR introduces an "Organizer Match" constraint to the Entity Collection Viewer filter system. It adds resource strings across multiple locales, extends the PropertyEntry model with an Options collection, implements conditional UI controls with visibility bindings, and adds constraint evaluation logic that queries OrganizerManager items. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Filter UI
participant ViewModel as FilterViewModel
participant OrganizerMgr as OrganizerManager
participant Item
User->>UI: Select Organizer from dropdown
UI->>ViewModel: Constraint with organizer name
ViewModel->>OrganizerMgr: GetInstance().Items[organizer name]
OrganizerMgr-->>ViewModel: OrganizerEntry
ViewModel->>Item: Check if item ID matches
alt Item found in organizer
Item-->>ViewModel: true (if Hue matches)
ViewModel-->>UI: Include in results
else Item not found
Item-->>ViewModel: false
ViewModel-->>UI: Exclude from results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (1)
ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterControl.xaml (1)
279-297: Collapse Options ComboBox when empty list to avoid empty controlCurrently hidden only when Options is null. If it’s an empty collection, users see an empty dropdown. Add a trigger for Count == 0.
Apply this XAML tweak:
<ComboBox Grid.Column="0" MinWidth="40" ItemsSource="{Binding Constraint.Options}" SelectedItem="{Binding Additional}" Width="{Binding ActualWidth, Converter={StaticResource CellWidthValueConverter}, ConverterParameter=15, ElementName=GridViewColumn, Mode=OneWay}"> <ComboBox.Style> <Style TargetType="{x:Type ComboBox}" BasedOn="{StaticResource {x:Type ComboBox}}"> <Setter Property="Visibility" Value="Visible" /> <Style.Triggers> <DataTrigger Binding="{Binding Constraint.Options}" Value="{x:Null}"> <Setter Property="Visibility" Value="Collapsed" /> </DataTrigger> + <DataTrigger Binding="{Binding Constraint.Options.Count}" Value="0"> + <Setter Property="Visibility" Value="Collapsed" /> + </DataTrigger> </Style.Triggers> </Style> </ComboBox.Style> </ComboBox>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ClassicAssist.Shared/Resources/Strings.Designer.cs(1 hunks)ClassicAssist.Shared/Resources/Strings.en-AU.resx(2 hunks)ClassicAssist.Shared/Resources/Strings.en-GB.resx(2 hunks)ClassicAssist.Shared/Resources/Strings.resx(1 hunks)ClassicAssist/Data/Autoloot/PropertyEntry.cs(4 hunks)ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterControl.xaml(6 hunks)ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterViewModel.cs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ClassicAssist/Data/Autoloot/PropertyEntry.cs (1)
ClassicAssist/Data/Options.cs (1)
Options(22-773)
ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterViewModel.cs (3)
ClassicAssist.Shared/Misc/ExtensionMethods.cs (1)
AddSorted(35-50)ClassicAssist/Data/Autoloot/PropertyEntry.cs (1)
PropertyEntry(28-119)ClassicAssist/Data/Organizer/OrganizerEntry.cs (1)
OrganizerEntry(7-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
ClassicAssist.Shared/Resources/Strings.resx (1)
2089-2091: LGTM!The new "Organizer Match" resource entry is correctly formatted and positioned consistently with other entries in the file.
ClassicAssist.Shared/Resources/Strings.en-GB.resx (2)
1-1: Good cleanup removing the BOM.The XML declaration is now standard UTF-8 without the BOM, which improves file compatibility.
2086-2088: LGTM!The "Organizer Match" resource entry is correctly added and consistent with the base resource file.
ClassicAssist/Data/Autoloot/PropertyEntry.cs (1)
21-21: LGTM!The new Options property follows the existing pattern in the class, using SetProperty for proper change notification. The use of ObservableCollection is appropriate for UI binding scenarios and aligns with the MVVM architecture.
Also applies to: 36-36, 78-82
ClassicAssist.Shared/Resources/Strings.en-AU.resx (2)
1-1: Good cleanup removing the BOM.The XML declaration is now standard UTF-8 without the BOM, improving file consistency across locale files.
2086-2088: LGTM!The "Organizer Match" resource entry is correctly added and consistent with the base resource file. The localization support is properly maintained across all locale files.
ClassicAssist.Shared/Resources/Strings.Designer.cs (1)
3867-3875: Resource accessor addition looks correctAccessor name and key are consistent with existing patterns (e.g., Autoloot_Match). No issues.
ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterViewModel.cs (1)
21-21: Trivial importsAdded namespaces are expected for the new feature.
Also applies to: 30-30
ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterControl.xaml (3)
21-21: New converter namespace alias is fineAlias resolves to ClassicAssist.Shared UI converters; no issues.
64-71: Template/behaviour tweaks look goodLayout and behaviour parameter binding are consistent and safe.
Also applies to: 83-85
35-35: No issues found—converter semantics and binding logic are correct.The
NullToBooleanConverterreturnstruewhen the value is non-null andfalsewhen null. The binding at line 344 correctly uses this: whenConstraint.Optionsis non-null, the converter returns true, the DataTrigger matches, and the TextBox visibility is set to Collapsed. This behaviour is intentional and appropriate. Line 35 is simply the resource key declaration. The UI logic is sound.
| Constraints.AddSorted( new PropertyEntry | ||
| { | ||
| Name = Strings.Organizer_Match, | ||
| ConstraintType = PropertyType.PredicateWithValue, | ||
| AllowedOperators = AutolootAllowedOperators.Equal | AutolootAllowedOperators.NotEqual, | ||
| Predicate = ( item, entry ) => | ||
| { | ||
| if ( entry.Additional == null ) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| OrganizerEntry organizer = OrganizerManager.GetInstance().Items.FirstOrDefault( e => e.Name == entry.Additional ); | ||
|
|
||
| return organizer != null && organizer.Items.Any( e => e.ID == item.ID && ( e.Hue == -1 || e.Hue == item.Hue ) ); | ||
| }, | ||
| Options = new ObservableCollection<string>( OrganizerManager.GetInstance().Items?.Select( o => o.Name ) ?? new List<string>() ) | ||
| } ); |
There was a problem hiding this comment.
Organizer_Match ignores NotEqual; add operator handling and input guards
Currently NotEqual behaves like Equal. Also harden for empty Additional and a null singleton edge case.
Apply this diff:
Constraints.AddSorted( new PropertyEntry
{
Name = Strings.Organizer_Match,
ConstraintType = PropertyType.PredicateWithValue,
AllowedOperators = AutolootAllowedOperators.Equal | AutolootAllowedOperators.NotEqual,
Predicate = ( item, entry ) =>
{
- if ( entry.Additional == null )
- {
- return false;
- }
-
- OrganizerEntry organizer = OrganizerManager.GetInstance().Items.FirstOrDefault( e => e.Name == entry.Additional );
-
- return organizer != null && organizer.Items.Any( e => e.ID == item.ID && ( e.Hue == -1 || e.Hue == item.Hue ) );
+ if ( string.IsNullOrEmpty( entry.Additional ) )
+ {
+ return false;
+ }
+
+ var mgr = OrganizerManager.GetInstance();
+ var organizer = mgr?.Items?.FirstOrDefault( e => string.Equals( e.Name, entry.Additional, StringComparison.Ordinal ) );
+
+ if ( organizer == null )
+ {
+ // Missing organiser: treat as no match for both operators.
+ return false;
+ }
+
+ bool isMatch = organizer.Items.Any( e => e.ID == item.ID && ( e.Hue == -1 || e.Hue == item.Hue ) );
+ return entry.Operator == AutolootOperator.NotEqual ? !isMatch : isMatch;
},
- Options = new ObservableCollection<string>( OrganizerManager.GetInstance().Items?.Select( o => o.Name ) ?? new List<string>() )
+ Options = new ObservableCollection<string>(
+ ( OrganizerManager.GetInstance().Items?.Select( o => o.Name )
+ ?? new List<string>() ).OrderBy( n => n, StringComparer.InvariantCultureIgnoreCase ) )
} );Additionally, consider refreshing Options if organiser names can change at runtime; otherwise users must reopen the view to see new profiles.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Constraints.AddSorted( new PropertyEntry | |
| { | |
| Name = Strings.Organizer_Match, | |
| ConstraintType = PropertyType.PredicateWithValue, | |
| AllowedOperators = AutolootAllowedOperators.Equal | AutolootAllowedOperators.NotEqual, | |
| Predicate = ( item, entry ) => | |
| { | |
| if ( entry.Additional == null ) | |
| { | |
| return false; | |
| } | |
| OrganizerEntry organizer = OrganizerManager.GetInstance().Items.FirstOrDefault( e => e.Name == entry.Additional ); | |
| return organizer != null && organizer.Items.Any( e => e.ID == item.ID && ( e.Hue == -1 || e.Hue == item.Hue ) ); | |
| }, | |
| Options = new ObservableCollection<string>( OrganizerManager.GetInstance().Items?.Select( o => o.Name ) ?? new List<string>() ) | |
| } ); | |
| Constraints.AddSorted( new PropertyEntry | |
| { | |
| Name = Strings.Organizer_Match, | |
| ConstraintType = PropertyType.PredicateWithValue, | |
| AllowedOperators = AutolootAllowedOperators.Equal | AutolootAllowedOperators.NotEqual, | |
| Predicate = ( item, entry ) => | |
| { | |
| if ( string.IsNullOrEmpty( entry.Additional ) ) | |
| { | |
| return false; | |
| } | |
| var mgr = OrganizerManager.GetInstance(); | |
| var organizer = mgr?.Items?.FirstOrDefault( e => string.Equals( e.Name, entry.Additional, StringComparison.Ordinal ) ); | |
| if ( organizer == null ) | |
| { | |
| // Missing organiser: treat as no match for both operators. | |
| return false; | |
| } | |
| bool isMatch = organizer.Items.Any( e => e.ID == item.ID && ( e.Hue == -1 || e.Hue == item.Hue ) ); | |
| return entry.Operator == AutolootOperator.NotEqual ? !isMatch : isMatch; | |
| }, | |
| Options = new ObservableCollection<string>( | |
| ( OrganizerManager.GetInstance().Items?.Select( o => o.Name ) | |
| ?? new List<string>() ).OrderBy( n => n, StringComparer.InvariantCultureIgnoreCase ) ) | |
| } ); |
🤖 Prompt for AI Agents
In ClassicAssist/UI/Views/ECV/Filter/EntityCollectionFilterViewModel.cs around
lines 123-140, the Predicate treats NotEqual the same as Equal and lacks
null/empty guards; update the predicate to first return false if entry or
entry.Additional is null or whitespace, guard against
OrganizerManager.GetInstance() or its Items being null before querying, then
resolve the organizer and implement operator-aware logic: for Equal return true
when a matching organizer item exists (ID and hue check), for NotEqual return
true when no such matching organizer item exists; also ensure Options is
initialized safely from OrganizerManager.GetInstance().Items (null-coalescing to
empty list) and add or call a refresh mechanism to rebuild Options when
organizer names change at runtime.
1 participant
Adds a new filter option to the Entity Collection View (ECV) that allows filtering based on items present in specified Organizer profiles. This allows users to easily find items that match their Organizer rules.
Summary by CodeRabbit