EES-5848 Remove default / auto-select filter option from public API #5605
Conversation
This file contains 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
ntsim
requested review from
leeoades,
mmoalin,
duncan-at-hiveit,
tomjonesdev and
jack-hive
mmoalin
approved these changes
Feb 18, 2025
This flag is not needed currently, but is also the incorrect implementation as it is specific to table tool. Anything related to the public API should be agnostic in relation to its consumers.
This previously did not correctly set `IsAggregate` to true for the 'Total' filter option.
This is purely for convenience to make it easier to run the script directly from the IDE.
This correctly implements default filter options in the public API using the consumer-agnostic terminology 'default' instead of 'auto-select'. Additionally, we add referential integrity and can determine the default option more easily by using an ID instead of a label string.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR removes the
IsAutoSelectfrom the public API'sFilterMetaViewModelso this information is no longer directly available to consumers (for now).It should be noted that the 'auto-select' option (referenced as
FilterMeta.AutoSelectLabel) is still stored in the database, but it has been renamed to the more consumer-agnosticFilterMeta.DefaultFilterOptionId(not directly tied to table tool).We have made
DefaultFilterOptionIdan ID that references theFilterOptionMetatable to gain referential integrity and allow for faster lookup of the default option in the future.Changes to
AssertDeepEqualToThe
AssertDeepEqualToextension was previously performing unnecessary inequality checks after checking for deep equality. This was quite awkward as it was causing various failures inImportMetadataFunctionTeststhat would have been hard to resolve without their removal.We have now removed the inequality checks from
AssertDeepEqualToand renamednotEqualPropertiestoignorePropertiesto make this distinction clearer.If you want to be certain that the object you're checking has specific properties that are unequal, the recommended approach is to just add those checks in yourself after the
AssertDeepEqualTo. A good example is in release content cloning tests where we check that the cloned objects have newCreateddates.As part of this, we've also switched any usages of
AssertExtensions.ExceptforignorePropertiesto much terser C# 8 collection expressions.Other changes
EES4735_RenameIsAggregateToIsAutoSelectmigration. We don't typically roll back migrations, but it irked me that this wasn't correctly implemented in case we ever needed to.