fix(filters): correct null matching logic [CW-5741] (#12627)
This PR fixes a bug came from assuming the old null check only mattered for the `is_not_present` filter. The fix keeps `not_equal_to` working but lets each operator decide what to do with `null`. Presence filters look at a shared `isNullish` flag, text filters still rely on `contains`, and date filters skip conversations with no timestamp. The new spec covers the null-assignee scenario for both `equal_to` and `not_equal_to` so we don’t miss this again.
This commit is contained in:
@@ -154,7 +154,10 @@ const equalTo = (filterValue, conversationValue) => {
|
||||
* It only works with string values and returns false for non-string types.
|
||||
*/
|
||||
const contains = (filterValue, conversationValue) => {
|
||||
if (typeof conversationValue === 'string') {
|
||||
if (
|
||||
typeof conversationValue === 'string' &&
|
||||
typeof filterValue === 'string'
|
||||
) {
|
||||
return conversationValue.toLowerCase().includes(filterValue.toLowerCase());
|
||||
}
|
||||
return false;
|
||||
@@ -190,10 +193,8 @@ const compareDates = (conversationValue, filterValue, compareFn) => {
|
||||
const matchesCondition = (conversationValue, filter) => {
|
||||
const { filter_operator: filterOperator, values } = filter;
|
||||
|
||||
// Handle null/undefined values
|
||||
if (conversationValue === null || conversationValue === undefined) {
|
||||
return filterOperator === 'is_not_present';
|
||||
}
|
||||
const isNullish =
|
||||
conversationValue === null || conversationValue === undefined;
|
||||
|
||||
const filterValue = Array.isArray(values)
|
||||
? values.map(resolveValue)
|
||||
@@ -213,10 +214,10 @@ const matchesCondition = (conversationValue, filter) => {
|
||||
return !contains(filterValue, conversationValue);
|
||||
|
||||
case 'is_present':
|
||||
return true; // We already handled null/undefined above
|
||||
return !isNullish;
|
||||
|
||||
case 'is_not_present':
|
||||
return false; // We already handled null/undefined above
|
||||
return isNullish;
|
||||
|
||||
case 'is_greater_than':
|
||||
return compareDates(conversationValue, filterValue, (a, b) => a > b);
|
||||
@@ -225,6 +226,10 @@ const matchesCondition = (conversationValue, filter) => {
|
||||
return compareDates(conversationValue, filterValue, (a, b) => a < b);
|
||||
|
||||
case 'days_before': {
|
||||
if (isNullish) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const today = new Date();
|
||||
const daysInMilliseconds = filterValue * 24 * 60 * 60 * 1000;
|
||||
const targetDate = new Date(today.getTime() - daysInMilliseconds);
|
||||
|
||||
@@ -192,6 +192,32 @@ describe('filterHelpers', () => {
|
||||
expect(matchesFilters(conversation, filters)).toBe(true);
|
||||
});
|
||||
|
||||
it('should not match conversation with equal_to operator when assignee is null', () => {
|
||||
const conversation = { meta: { assignee: null } };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'assignee_id',
|
||||
filter_operator: 'equal_to',
|
||||
values: { id: 1, name: 'John Doe' },
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(false);
|
||||
});
|
||||
|
||||
it('should match conversation with not_equal_to operator when assignee is null', () => {
|
||||
const conversation = { meta: { assignee: null } };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'assignee_id',
|
||||
filter_operator: 'not_equal_to',
|
||||
values: { id: 1, name: 'John Doe' },
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(true);
|
||||
});
|
||||
|
||||
it('should match conversation with is_not_present operator for assignee_id', () => {
|
||||
const conversation = { meta: { assignee: null } };
|
||||
const filters = [
|
||||
@@ -285,6 +311,58 @@ describe('filterHelpers', () => {
|
||||
expect(matchesFilters(conversation, filters)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match contains operator when display_id is null', () => {
|
||||
const conversation = { id: null };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'display_id',
|
||||
filter_operator: 'contains',
|
||||
values: '234',
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match contains operator when filter value is null', () => {
|
||||
const conversation = { id: '12345' };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'display_id',
|
||||
filter_operator: 'contains',
|
||||
values: null,
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(false);
|
||||
});
|
||||
|
||||
it('should match does_not_contain operator when display_id is null', () => {
|
||||
const conversation = { id: null };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'display_id',
|
||||
filter_operator: 'does_not_contain',
|
||||
values: '234',
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(true);
|
||||
});
|
||||
|
||||
it('should match does_not_contain operator when filter value is null', () => {
|
||||
const conversation = { id: '12345' };
|
||||
const filters = [
|
||||
{
|
||||
attribute_key: 'display_id',
|
||||
filter_operator: 'does_not_contain',
|
||||
values: null,
|
||||
query_operator: 'and',
|
||||
},
|
||||
];
|
||||
expect(matchesFilters(conversation, filters)).toBe(true);
|
||||
});
|
||||
|
||||
it('should match conversation with does_not_contain operator when value is not present', () => {
|
||||
const conversation = { id: '12345' };
|
||||
const filters = [
|
||||
|
||||
Reference in New Issue
Block a user