From a3e997137919c8784384e409558c3861481a5237 Mon Sep 17 00:00:00 2001
From: Neil <55785687+carkom@users.noreply.github.com>
Date: Thu, 20 Jul 2023 17:36:37 +0100
Subject: [PATCH] Add "not" checkbox for string filters (#1287)
---
.../e2e/page-models/rules-page.js | 2 +-
.../src/components/ManageRules.js | 2 +-
.../src/components/common/Select.tsx | 55 +++++++++++---
.../src/components/filters/FiltersMenu.js | 76 ++++++++++++++-----
.../src/components/modals/EditRule.js | 22 +++++-
.../loot-core/src/server/accounts/rules.ts | 61 ++++++++++++---
.../src/server/accounts/transaction-rules.ts | 23 +++++-
packages/loot-core/src/server/aql/compiler.ts | 25 ++++++
packages/loot-core/src/shared/rules.ts | 10 ++-
upcoming-release-notes/1287.md | 6 ++
10 files changed, 233 insertions(+), 49 deletions(-)
create mode 100644 upcoming-release-notes/1287.md
diff --git a/packages/desktop-client/e2e/page-models/rules-page.js b/packages/desktop-client/e2e/page-models/rules-page.js
index 17d36257b..31926bcec 100644
--- a/packages/desktop-client/e2e/page-models/rules-page.js
+++ b/packages/desktop-client/e2e/page-models/rules-page.js
@@ -81,7 +81,7 @@ export class RulesPage {
if (op) {
await row.getByRole('button', { name: 'is' }).click();
- await this.page.getByRole('option', { name: op }).click();
+ await this.page.getByRole('option', { name: op, exact: true }).click();
}
if (value) {
diff --git a/packages/desktop-client/src/components/ManageRules.js b/packages/desktop-client/src/components/ManageRules.js
index 5a26a7bd5..5f724971e 100644
--- a/packages/desktop-client/src/components/ManageRules.js
+++ b/packages/desktop-client/src/components/ManageRules.js
@@ -548,7 +548,7 @@ function ruleToString(rule, data) {
let conditions = rule.conditions.flatMap(cond => [
mapField(cond.field),
friendlyOp(cond.op),
- cond.op === 'oneOf'
+ cond.op === 'oneOf' || cond.op === 'notOneOf'
? cond.value.map(v => mapValue(cond.field, v, data)).join(', ')
: mapValue(cond.field, cond.value, data),
]);
diff --git a/packages/desktop-client/src/components/common/Select.tsx b/packages/desktop-client/src/components/common/Select.tsx
index 8f9b85e88..f328d8479 100644
--- a/packages/desktop-client/src/components/common/Select.tsx
+++ b/packages/desktop-client/src/components/common/Select.tsx
@@ -8,6 +8,7 @@ import {
import { type CSSProperties, css } from 'glamor';
import ExpandArrow from '../../icons/v0/ExpandArrow';
+import { colors } from '../../style';
type CustomSelectProps = {
options: Array<[string, string]>;
@@ -16,6 +17,7 @@ type CustomSelectProps = {
onChange?: (newValue: string) => void;
style?: CSSProperties;
wrapperStyle?: CSSProperties;
+ line: number;
disabledKeys?: string[];
};
@@ -41,6 +43,7 @@ export default function Select({
onChange,
style,
wrapperStyle,
+ line,
disabledKeys = [],
}: CustomSelectProps) {
const arrowSize = 7;
@@ -73,17 +76,47 @@ export default function Select({
</span>
</ListboxButton>
<ListboxPopover style={{ zIndex: 10000, outline: 0, borderRadius: 4 }}>
- <ListboxList style={{ maxHeight: 250, overflowY: 'auto' }}>
- {options.map(([value, label]) => (
- <ListboxOption
- key={value}
- value={value}
- disabled={disabledKeys.includes(value)}
- >
- {label}
- </ListboxOption>
- ))}
- </ListboxList>
+ {!line ? (
+ <ListboxList style={{ maxHeight: 250, overflowY: 'auto' }}>
+ {options.map(([value, label]) => (
+ <ListboxOption
+ key={value}
+ value={value}
+ disabled={disabledKeys.includes(value)}
+ >
+ {label}
+ </ListboxOption>
+ ))}
+ </ListboxList>
+ ) : (
+ <ListboxList style={{ maxHeight: 250, overflowY: 'auto' }}>
+ {options.slice(0, line).map(([value, label]) => (
+ <ListboxOption
+ key={value}
+ value={value}
+ disabled={disabledKeys.includes(value)}
+ >
+ {label}
+ </ListboxOption>
+ ))}
+ <div
+ style={{
+ padding: '2px',
+ marginTop: 5,
+ borderTop: '1px solid ' + colors.n10,
+ }}
+ />
+ {options.slice(line, options.length).map(([value, label]) => (
+ <ListboxOption
+ key={value}
+ value={value}
+ disabled={disabledKeys.includes(value)}
+ >
+ {label}
+ </ListboxOption>
+ ))}
+ </ListboxList>
+ )}
</ListboxPopover>
</ListboxInput>
);
diff --git a/packages/desktop-client/src/components/filters/FiltersMenu.js b/packages/desktop-client/src/components/filters/FiltersMenu.js
index c44d39ab0..cea87325f 100644
--- a/packages/desktop-client/src/components/filters/FiltersMenu.js
+++ b/packages/desktop-client/src/components/filters/FiltersMenu.js
@@ -109,9 +109,15 @@ function updateFilterReducer(state, action) {
case 'set-op': {
let type = FIELD_TYPES.get(state.field);
let value = state.value;
- if (type === 'id' && action.op === 'contains') {
- // Clear out the value if switching between contains for
- // the id type
+ if (
+ (type === 'id' || type === 'string') &&
+ (action.op === 'contains' ||
+ action.op === 'is' ||
+ action.op === 'doesNotContain' ||
+ action.op === 'isNot')
+ ) {
+ // Clear out the value if switching between contains or
+ // is/oneof for the id or string type
value = null;
}
return { ...state, op: action.op, value };
@@ -159,7 +165,7 @@ function ConfigureField({
<Tooltip
position="bottom-left"
style={{ padding: 15 }}
- width={250}
+ width={275}
onClose={() => dispatch({ type: 'close' })}
>
<FocusScope>
@@ -232,14 +238,40 @@ function ConfigureField({
}}
/>,
]
- : ops.map(currOp => (
- <OpButton
- key={currOp}
- op={currOp}
- selected={currOp === op}
- onClick={() => dispatch({ type: 'set-op', op: currOp })}
- />
- ))}
+ : [
+ <Stack
+ direction="row"
+ align="flex-start"
+ spacing={1}
+ style={{ flexWrap: 'wrap' }}
+ >
+ {ops.slice(0, 3).map(currOp => (
+ <OpButton
+ key={currOp}
+ op={currOp}
+ selected={currOp === op}
+ onClick={() => dispatch({ type: 'set-op', op: currOp })}
+ />
+ ))}
+ </Stack>,
+ <Stack
+ direction="row"
+ align="flex-start"
+ spacing={1}
+ style={{ flexWrap: 'wrap' }}
+ >
+ {ops.slice(3, ops.length).map(currOp => (
+ <View>
+ <OpButton
+ key={currOp}
+ op={currOp}
+ selected={currOp === op}
+ onClick={() => dispatch({ type: 'set-op', op: currOp })}
+ />
+ </View>
+ ))}
+ </Stack>,
+ ]}
</Stack>
<form action="#">
@@ -248,19 +280,27 @@ function ConfigureField({
inputRef={inputRef}
field={field}
subfield={subfield}
- type={type === 'id' && op === 'contains' ? 'string' : type}
+ type={
+ type === 'id' && (op === 'contains' || op === 'doesNotContain')
+ ? 'string'
+ : type
+ }
value={value}
- multi={op === 'oneOf'}
+ multi={op === 'oneOf' || op === 'notOneOf'}
style={{ marginTop: 10 }}
onChange={v => dispatch({ type: 'set-value', value: v })}
/>
)}
- <Stack direction="row" justify="flex-end" align="center">
+ <Stack
+ direction="row"
+ justify="flex-end"
+ align="center"
+ style={{ marginTop: 15 }}
+ >
<View style={{ flex: 1 }} />
<Button
primary
- style={{ marginTop: 15 }}
onClick={e => {
e.preventDefault();
onApply({
@@ -472,12 +512,12 @@ function FilterExpression({
<Text style={{ color: colors.p4 }}>
{mapField(field, options)}
</Text>{' '}
- <Text style={{ color: colors.n3 }}>{friendlyOp(op)}</Text>{' '}
+ <Text style={{ color: colors.n3 }}>{friendlyOp(op, null)}</Text>{' '}
<Value
value={value}
field={field}
inline={true}
- valueIsRaw={op === 'contains'}
+ valueIsRaw={op === 'contains' || op === 'doesNotContain'}
/>
</>
)}
diff --git a/packages/desktop-client/src/components/modals/EditRule.js b/packages/desktop-client/src/components/modals/EditRule.js
index c9733abc7..c3b40802d 100644
--- a/packages/desktop-client/src/components/modals/EditRule.js
+++ b/packages/desktop-client/src/components/modals/EditRule.js
@@ -91,10 +91,15 @@ export function OpSelect({
formatOp = friendlyOp,
onChange,
}) {
+ let line;
// We don't support the `contains` operator for the id type for
// rules yet
if (type === 'id') {
- ops = ops.filter(op => op !== 'contains');
+ ops = ops.filter(op => op !== 'contains' && op !== 'doesNotContain');
+ line = ops.length / 2;
+ }
+ if (type === 'string') {
+ line = ops.length / 2;
}
return (
@@ -102,6 +107,7 @@ export function OpSelect({
options={ops.map(op => [op, formatOp(op, type)])}
value={value}
onChange={value => onChange('op', value)}
+ line={line}
style={style}
/>
);
@@ -200,7 +206,7 @@ function ConditionEditor({
field={field}
type={type}
value={value}
- multi={op === 'oneOf'}
+ multi={op === 'oneOf' || op === 'notOneOf'}
onChange={v => onChange('value', v)}
/>
);
@@ -471,14 +477,22 @@ function ConditionsList({
// Switching between oneOf and other operators is a
// special-case. It changes the input type, so we need to
// clear the value
- if (cond.op !== 'oneOf' && op === 'oneOf') {
+ if (
+ cond.op !== 'oneOf' &&
+ cond.op !== 'notOneOf' &&
+ (op === 'oneOf' || op === 'notOneOf')
+ ) {
return newInput(
makeValue(cond.value != null ? [cond.value] : [], {
...cond,
op: value,
}),
);
- } else if (cond.op === 'oneOf' && op !== 'oneOf') {
+ } else if (
+ (cond.op === 'oneOf' || cond.op === 'notOneOf') &&
+ op !== 'oneOf' &&
+ op !== 'notOneOf'
+ ) {
return newInput(
makeValue(cond.value.length > 0 ? cond.value[0] : null, {
...cond,
diff --git a/packages/loot-core/src/server/accounts/rules.ts b/packages/loot-core/src/server/accounts/rules.ts
index d79ef88f8..87203e2c9 100644
--- a/packages/loot-core/src/server/accounts/rules.ts
+++ b/packages/loot-core/src/server/accounts/rules.ts
@@ -109,10 +109,10 @@ let CONDITION_TYPES = {
},
},
id: {
- ops: ['is', 'contains', 'oneOf'],
+ ops: ['is', 'contains', 'oneOf', 'isNot', 'doesNotContain', 'notOneOf'],
nullable: true,
parse(op, value, fieldName) {
- if (op === 'oneOf') {
+ if (op === 'oneOf' || op === 'notOneOf') {
assert(
Array.isArray(value),
'no-empty-array',
@@ -124,10 +124,10 @@ let CONDITION_TYPES = {
},
},
string: {
- ops: ['is', 'contains', 'oneOf'],
+ ops: ['is', 'contains', 'oneOf', 'isNot', 'doesNotContain', 'notOneOf'],
nullable: false,
parse(op, value, fieldName) {
- if (op === 'oneOf') {
+ if (op === 'oneOf' || op === 'notOneOf') {
assert(
Array.isArray(value),
'no-empty-array',
@@ -138,7 +138,7 @@ let CONDITION_TYPES = {
return value.filter(Boolean).map(val => val.toLowerCase());
}
- if (op === 'contains') {
+ if (op === 'contains' || op === 'doesNotContain') {
assert(
typeof value === 'string' && value.length > 0,
'no-empty-string',
@@ -323,8 +323,10 @@ export class Condition {
}
return fieldValue === number;
}
-
return fieldValue === this.value;
+
+ case 'isNot':
+ return fieldValue !== this.value;
case 'isbetween': {
// The parsing logic already checks that the value is of the
// right type (only numbers with high and low)
@@ -336,11 +338,21 @@ export class Condition {
return false;
}
return fieldValue.indexOf(this.value) !== -1;
+ case 'doesNotContain':
+ if (fieldValue === null) {
+ return false;
+ }
+ return fieldValue.indexOf(this.value) === -1;
case 'oneOf':
if (fieldValue === null) {
return false;
}
return this.value.indexOf(fieldValue) !== -1;
+ case 'notOneOf':
+ if (fieldValue === null) {
+ return false;
+ }
+ return this.value.indexOf(fieldValue) === -1;
case 'gt':
if (fieldValue === null) {
return false;
@@ -568,8 +580,14 @@ export class RuleIndexer {
let cond = rule.conditions.find(cond => cond.field === this.field);
let indexes = [];
- if (cond && (cond.op === 'oneOf' || cond.op === 'is')) {
- if (cond.op === 'oneOf') {
+ if (
+ cond &&
+ (cond.op === 'oneOf' ||
+ cond.op === 'is' ||
+ cond.op === 'isNot' ||
+ cond.op === 'notOneOf')
+ ) {
+ if (cond.op === 'oneOf' || cond.op === 'notOneOf') {
cond.value.forEach(val => indexes.push(this.getIndexForValue(val)));
} else {
indexes.push(this.getIndexForValue(cond.value));
@@ -635,7 +653,12 @@ function computeScore(rule) {
if (
rule.conditions.every(
- cond => cond.op === 'is' || cond.op === 'isapprox' || cond.op === 'oneOf',
+ cond =>
+ cond.op === 'is' ||
+ cond.op === 'isNot' ||
+ cond.op === 'isapprox' ||
+ cond.op === 'oneOf' ||
+ cond.op === 'notOneOf',
)
) {
return initialScore * 2;
@@ -716,10 +739,18 @@ export function migrateIds(rule, mappings) {
cond.value = mappings.get(cond.rawValue) || cond.rawValue;
cond.unparsedValue = cond.value;
break;
+ case 'isNot':
+ cond.value = mappings.get(cond.rawValue) || cond.rawValue;
+ cond.unparsedValue = cond.value;
+ break;
case 'oneOf':
cond.value = cond.rawValue.map(v => mappings.get(v) || v);
cond.unparsedValue = [...cond.value];
break;
+ case 'notOneOf':
+ cond.value = cond.rawValue.map(v => mappings.get(v) || v);
+ cond.unparsedValue = [...cond.value];
+ break;
default:
}
}
@@ -750,6 +781,11 @@ export function iterateIds(rules, fieldName, func) {
continue ruleiter;
}
break;
+ case 'isNot':
+ if (func(rule, cond.value)) {
+ continue ruleiter;
+ }
+ break;
case 'oneOf':
for (let vi = 0; vi < cond.value.length; vi++) {
if (func(rule, cond.value[vi])) {
@@ -757,6 +793,13 @@ export function iterateIds(rules, fieldName, func) {
}
}
break;
+ case 'notOneOf':
+ for (let vi = 0; vi < cond.value.length; vi++) {
+ if (func(rule, cond.value[vi])) {
+ continue ruleiter;
+ }
+ }
+ break;
default:
}
}
diff --git a/packages/loot-core/src/server/accounts/transaction-rules.ts b/packages/loot-core/src/server/accounts/transaction-rules.ts
index 9190bc589..56cc6a73a 100644
--- a/packages/loot-core/src/server/accounts/transaction-rules.ts
+++ b/packages/loot-core/src/server/accounts/transaction-rules.ts
@@ -416,8 +416,9 @@ export function conditionsToAQL(conditions, { recurDateBounds = 100 } = {}) {
}
return apply(field, '$eq', number);
}
-
return apply(field, '$eq', value);
+ case 'isNot':
+ return apply(field, '$ne', value);
case 'isbetween':
// This operator is only applicable to the specific `between`
@@ -434,6 +435,14 @@ export function conditionsToAQL(conditions, { recurDateBounds = 100 } = {}) {
'$like',
'%' + value + '%',
);
+ case 'doesNotContain':
+ // Running contains with id will automatically reach into
+ // the `name` of the referenced table and do a string match
+ return apply(
+ type === 'id' ? field + '.name' : field,
+ '$notlike',
+ '%' + value + '%',
+ );
case 'oneOf':
let values = value;
if (values.length === 0) {
@@ -441,6 +450,13 @@ export function conditionsToAQL(conditions, { recurDateBounds = 100 } = {}) {
return { id: null };
}
return { $or: values.map(v => apply(field, '$eq', v)) };
+ case 'notOneOf':
+ let notValues = value;
+ if (notValues.length === 0) {
+ // This forces it to match nothing
+ return { id: null };
+ }
+ return { $and: notValues.map(v => apply(field, '$ne', v)) };
case 'gt':
return apply(field, '$gt', getValue(value));
case 'gte':
@@ -527,7 +543,7 @@ function* getIsSetterRules(
rule.actions[0].field === actionField &&
(actionValue === undefined || rule.actions[0].value === actionValue) &&
rule.conditions.length === 1 &&
- rule.conditions[0].op === 'is' &&
+ (rule.conditions[0].op === 'is' || rule.conditions[0].op === 'isNot') &&
rule.conditions[0].field === condField &&
(condValue === undefined || rule.conditions[0].value === condValue)
) {
@@ -555,7 +571,8 @@ function* getOneOfSetterRules(
rule.actions[0].field === actionField &&
(actionValue == null || rule.actions[0].value === actionValue) &&
rule.conditions.length === 1 &&
- rule.conditions[0].op === 'oneOf' &&
+ (rule.conditions[0].op === 'oneOf' ||
+ rule.conditions[0].op === 'oneOf') &&
rule.conditions[0].field === condField &&
(condValue == null || rule.conditions[0].value.indexOf(condValue) !== -1)
) {
diff --git a/packages/loot-core/src/server/aql/compiler.ts b/packages/loot-core/src/server/aql/compiler.ts
index 4898e9715..03281504e 100644
--- a/packages/loot-core/src/server/aql/compiler.ts
+++ b/packages/loot-core/src/server/aql/compiler.ts
@@ -674,6 +674,27 @@ const compileOp = saveStack('op', (state, fieldRef, opData) => {
return `${left} = ${right}`;
}
+ case '$ne': {
+ if (castInput(state, rhs, lhs.type).type === 'null') {
+ return `${val(state, lhs)} IS NULL`;
+ }
+
+ let [left, right] = valArray(state, [lhs, rhs], [null, lhs.type]);
+
+ if (rhs.type === 'param') {
+ let orders = state.namedParameters.map(param => {
+ return param === rhs || param === lhs ? [param, { ...param }] : param;
+ });
+ state.namedParameters = [].concat.apply([], orders);
+
+ return `CASE
+ WHEN ${left} IS NULL THEN ${right} IS NULL
+ ELSE ${left} != ${right}
+ END`;
+ }
+
+ return `${left} != ${right}`;
+ }
case '$oneof': {
let [left, right] = valArray(state, [lhs, rhs], [null, 'array']);
// Dedupe the ids
@@ -685,6 +706,10 @@ const compileOp = saveStack('op', (state, fieldRef, opData) => {
let [left, right] = valArray(state, [lhs, rhs], ['string', 'string']);
return `${left} LIKE ${right}`;
}
+ case '$notlike': {
+ let [left, right] = valArray(state, [lhs, rhs], ['string', 'string']);
+ return `(${left} NOT LIKE ${right}\n OR ${left} IS NULL)`;
+ }
default:
throw new CompileError(`Unknown operator: ${op}`);
}
diff --git a/packages/loot-core/src/shared/rules.ts b/packages/loot-core/src/shared/rules.ts
index efcf73da0..aa6fb843c 100644
--- a/packages/loot-core/src/shared/rules.ts
+++ b/packages/loot-core/src/shared/rules.ts
@@ -8,7 +8,7 @@ export const TYPE_INFO = {
nullable: false,
},
id: {
- ops: ['is', 'contains', 'oneOf'],
+ ops: ['is', 'contains', 'oneOf', 'isNot', 'doesNotContain', 'notOneOf'],
nullable: true,
},
saved: {
@@ -16,7 +16,7 @@ export const TYPE_INFO = {
nullable: false,
},
string: {
- ops: ['is', 'contains', 'oneOf'],
+ ops: ['is', 'contains', 'oneOf', 'isNot', 'doesNotContain', 'notOneOf'],
nullable: false,
},
number: {
@@ -71,14 +71,20 @@ export function friendlyOp(op, type) {
switch (op) {
case 'oneOf':
return 'one of';
+ case 'notOneOf':
+ return 'not one of';
case 'is':
return 'is';
+ case 'isNot':
+ return 'is not';
case 'isapprox':
return 'is approx';
case 'isbetween':
return 'is between';
case 'contains':
return 'contains';
+ case 'doesNotContain':
+ return 'does not contain';
case 'gt':
if (type === 'date') {
return 'is after';
diff --git a/upcoming-release-notes/1287.md b/upcoming-release-notes/1287.md
new file mode 100644
index 000000000..fd0048fb9
--- /dev/null
+++ b/upcoming-release-notes/1287.md
@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [carkom]
+---
+
+Added a negate options to the filters that are string based fields. This was added to Accounts page filters as well as the rules modal.
\ No newline at end of file
--
GitLab