feat: allow comparing list sizes against a number #1530
Conversation
| type: RuleType.NUMBER, | ||
| type: RuleType.NUMBER_LIST, | ||
| }, | ||
| { | ||
| id: 9, | ||
| name: 'fileQuality', | ||
| humanName: '[list] File - quality (2160, 1080,..)', | ||
| mediaType: MediaType.MOVIE, | ||
| type: RuleType.NUMBER, | ||
| type: RuleType.NUMBER_LIST, |
There was a problem hiding this comment.
It turns out that the getters for these just return a number so the comparison currently returns false as they aren't arrays. The API response types are set to number, but I'm sure you could have multiple audio channels. I'll have a dig into the Radarr API and see what's going on with that one. fileQuality will only ever be a single number as you can't have multiple files so that one needs to be set to RuleType.NUMBER.
I actually think we'll need to re-include contains for RuleType.NUMBER as it's no longer possible to compare to a list. E.g. you may want to target qualities of 720, 1080. We'll have to fix the scenarios I mentioned in my other comment for that too. Hmm scratch that, as it was a number you could only provide a number, not a list. That's a bit of a limitation we could fix at some point though. I suspect people are working around this by including multiple rules.
There was a problem hiding this comment.
I can confirm that Radarr will only ever serve up a single number for audio channels, so that one can go back to RuleType.NUMBER now. I'm going to check all of the other [list]'s to see if we've got any more imposters...
There was a problem hiding this comment.
Forgot to come back to this, all of the other [list]'s are fine.
| ], | ||
| 'number list', | ||
| ); | ||
| static readonly TEXT_LIST = new RuleType( |
There was a problem hiding this comment.
I just found that it's currently not possible to save a custom value for these types, the API returns Validation failed.
There's a few conditionals in ui\src\components\Rules\Rule\RuleCreator\RuleInput\index.tsx that likely need updating to handle the new types.
There's also a couple of functions that switch on the types that probably need updating as well:
See server\src\modules\rules\constants\constants.service.ts
getCustomValueIdentifier
getCustomValueFromIdentifier
There was a problem hiding this comment.
aha good catch, thanks, just realised I didn't fully test end-to-end yet - only to the point of selecting it in the UI. I'll also actually run it on some rules and make sure it works...
| @@ -372,7 +413,7 @@ export class RuleConstants { | |||
| name: 'tags', | |||
| humanName: '[list] Tags (Text if 1, otherwise list)', | |||
There was a problem hiding this comment.
| humanName: '[list] Tags (Text if 1, otherwise list)', | |
| humanName: '[list] Tags', |
This seems to always return a list even if there's 1 result, may have been a functionality change in the past.
This PR:
NUMBER_LISTandTEXT_LIST(I chose "list" to align with the term already used in the UI/copy)COUNT_EQUALS,COUNT_NOT_EQUALS,COUNT_BIGGER,COUNT_SMALLERNUMBERandTEXTto the new rule typesdoRuleActionto support the new potential value types. I want to do a lot more cleanup here, but I'll do that in its own refactoring PR instead.I think this requires no migration, since all rule actions that could have been applicable to list values previously are still valid on the new rule types (it is a superset). But I'm not 100% sure if I'm looking at the right place for how the rule actions are serialised and persisted.