diff --git a/pkg/models/task_collection_test.go b/pkg/models/task_collection_test.go index 2ddf7f4d9..be096381e 100644 --- a/pkg/models/task_collection_test.go +++ b/pkg/models/task_collection_test.go @@ -1263,6 +1263,48 @@ func TestTaskCollection_ReadAll(t *testing.T) { }, wantErr: false, }, + { + name: "filter labels with nulls", + fields: fields{ + Filter: "labels = 5", + FilterIncludeNulls: true, + }, + args: defaultArgs, + want: []*Task{ + task3, + task4, + task5, + task6, + task7, + task8, + task9, + task10, + task11, + task12, + task15, + task16, + task17, + task18, + task19, + task20, + task21, + task22, + task23, + task24, + task25, + task26, + task27, + task28, + task29, + task30, + task31, + task32, + task33, + task35, + task39, + }, + wantErr: false, + }, { name: "filter labels not eq", fields: fields{ @@ -1272,8 +1314,38 @@ func TestTaskCollection_ReadAll(t *testing.T) { want: []*Task{ task1, task2, + task3, + task4, + task5, + task6, + task7, + task8, + task9, + task10, + task11, + task12, + task15, + task16, + task17, + task18, + task19, + task20, + task21, + task22, + task23, + task24, + task25, + task26, + task27, + task28, + task29, + task30, + task31, + task32, + task33, //task35, // task 35 has a label 5 and 4 + task39, }, wantErr: false, }, @@ -1286,8 +1358,38 @@ func TestTaskCollection_ReadAll(t *testing.T) { want: []*Task{ task1, task2, + task3, + task4, + task5, + task6, + task7, + task8, + task9, + task10, + task11, + task12, + task15, + task16, + task17, + task18, + task19, + task20, + task21, + task22, + task23, + task24, + task25, + task26, + task27, + task28, + task29, + task30, + task31, + task32, + task33, //task35, // task 35 has a label 5 and 4 + task39, }, wantErr: false, }, diff --git a/pkg/models/task_search.go b/pkg/models/task_search.go index f289c1d99..884abaf9d 100644 --- a/pkg/models/task_search.go +++ b/pkg/models/task_search.go @@ -34,6 +34,61 @@ import ( "xorm.io/xorm/schemas" ) +type SubTableFilter struct { + Table string + BaseFilter string + FilterableField string + AllowNullCheck bool +} + +type SubTableFilters map[string]SubTableFilter + +var subTableFilters = SubTableFilters{ + "labels": { + Table: "label_tasks", + BaseFilter: "tasks.id = task_id", + FilterableField: "label_id", + AllowNullCheck: true, + }, + "label_id": { + Table: "label_tasks", + BaseFilter: "tasks.id = task_id", + FilterableField: "label_id", + AllowNullCheck: true, + }, + "reminders": { + Table: "task_reminders", + BaseFilter: "tasks.id = task_id", + FilterableField: "reminder", + AllowNullCheck: true, + }, + "assignees": { + Table: "task_assignees", + BaseFilter: "tasks.id = task_id", + FilterableField: "username", + AllowNullCheck: true, + }, + "parent_project": { + Table: "projects", + BaseFilter: "tasks.project_id = id", + FilterableField: "parent_project_id", + AllowNullCheck: false, + }, + "parent_project_id": { + Table: "projects", + BaseFilter: "tasks.project_id = id", + FilterableField: "parent_project_id", + AllowNullCheck: false, + }, +} + +var strictComparators = map[taskFilterComparator]bool{ + taskFilterComparatorIn: true, + taskFilterComparatorNotIn: true, + taskFilterComparatorEquals: true, + taskFilterComparatorNotEquals: true, +} + type taskSearcher interface { Search(opts *taskSearchOptions) (tasks []*Task, totalCount int64, err error) } @@ -44,6 +99,20 @@ type dbTaskSearcher struct { hasFavoritesProject bool } +func (sf *SubTableFilter) ToBaseSubQuery() *builder.Builder { + var cond = builder. + Select("1"). + From(sf.Table). + Where(builder.Expr(sf.BaseFilter)) + + // little hack to add users table for assignees filter + if sf.Table == "task_assignees" { + cond.Join("INNER", "users", "users.id = user_id") + } + + return cond +} + func getOrderByDBStatement(opts *taskSearchOptions) (orderby string, err error) { // Since xorm does not use placeholders for order by, it is possible to expose this with sql injection if we're directly // passing user input to the db. @@ -89,7 +158,6 @@ func getOrderByDBStatement(opts *taskSearchOptions) (orderby string, err error) func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) (filterCond builder.Cond, err error) { var dbFilters = make([]builder.Cond, 0, len(rawFilters)) - var addLabelInFilter = false // To still find tasks with nil values, we exclude 0s when comparing with >/< values. for _, f := range rawFilters { @@ -102,103 +170,45 @@ func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) ( continue } - if f.field == "reminders" { - filter, err := getFilterCond(&taskFilter{ - // recreating the struct here to avoid modifying it when reusing the opts struct - field: "reminder", - value: f.value, - comparator: f.comparator, - isNumeric: f.isNumeric, - }, includeNulls) - if err != nil { - return nil, err - } - dbFilters = append(dbFilters, getFilterCondForSeparateTable("task_reminders", filter)) - continue - } - - if f.field == "assignees" { - if f.comparator == taskFilterComparatorLike { - return - } - filter, err := getFilterCond(&taskFilter{ - // recreating the struct here to avoid modifying it when reusing the opts struct - field: "username", - value: f.value, - comparator: f.comparator, - isNumeric: f.isNumeric, - }, includeNulls) - if err != nil { - return nil, err - } - - assigneeFilter := builder.In("user_id", - builder.Select("id"). - From("users"). - Where(filter), - ) - dbFilters = append(dbFilters, getFilterCondForSeparateTable("task_assignees", assigneeFilter)) - continue - } - - if f.field == "labels" || f.field == "label_id" { - filter, err := getFilterCond(&taskFilter{ - // recreating the struct here to avoid modifying it when reusing the opts struct - field: "label_id", - value: f.value, - comparator: f.comparator, - isNumeric: f.isNumeric, - }, includeNulls) - if err != nil { - return nil, err - } - - if f.comparator == taskFilterComparatorNotEquals { - dbFilters = append(dbFilters, getNegativeFilterCondForSeparateTable( - "label_tasks", - builder.Eq{"label_id": f.value}, - )) - if !includeNulls { - addLabelInFilter = true - } + subTableFilterParams, ok := subTableFilters[f.field] + if ok { + if f.field == "assignees" && (f.comparator == taskFilterComparatorLike) { continue } - if f.comparator == taskFilterComparatorNotIn { - dbFilters = append(dbFilters, getNegativeFilterCondForSeparateTable( - "label_tasks", - builder.In("label_id", f.value), - )) - if !includeNulls { - addLabelInFilter = true - } - continue + comparator := f.comparator + _, ok = strictComparators[f.comparator] + // we will select all specified values in both cases, negative and positive filtering. + // but later we will eather check their existence or absence of them. + if ok { + comparator = taskFilterComparatorIn } - dbFilters = append(dbFilters, getFilterCondForSeparateTable("label_tasks", filter)) - continue - } - - if f.field == "parent_project" || f.field == "parent_project_id" { filter, err := getFilterCond(&taskFilter{ // recreating the struct here to avoid modifying it when reusing the opts struct - field: "parent_project_id", + field: subTableFilterParams.FilterableField, value: f.value, - comparator: f.comparator, + comparator: comparator, isNumeric: f.isNumeric, - }, includeNulls) + }, false) if err != nil { return nil, err } - cond := builder.In( - "project_id", - builder. - Select("id"). - From("projects"). - Where(filter), - ) - dbFilters = append(dbFilters, cond) + filterSubQuery := subTableFilterParams.ToBaseSubQuery().And(filter) + + if f.comparator == taskFilterComparatorNotEquals || f.comparator == taskFilterComparatorNotIn { + filter = builder.NotExists(filterSubQuery) + } else { + filter = builder.Exists(filterSubQuery) + } + + if includeNulls && subTableFilterParams.AllowNullCheck { + // check that we have no any connected values for this field + filter = builder.Or(filter, builder.NotExists(subTableFilterParams.ToBaseSubQuery())) + } + + dbFilters = append(dbFilters, filter) continue } @@ -229,16 +239,6 @@ func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) ( } } } - - if addLabelInFilter { - filterCond = builder.And( - filterCond, - builder.In("tasks.id", builder. - Select("task_id"). - From("label_tasks"), - ), - ) - } } return filterCond, nil diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 36754299d..e7562fe47 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -260,26 +260,6 @@ func getFilterCond(f *taskFilter, includeNulls bool) (cond builder.Cond, err err return } -func getNegativeFilterCondForSeparateTable(table string, cond builder.Cond) builder.Cond { - return builder.NotIn( - "tasks.id", - builder. - Select("task_id"). - From(table). - Where(cond), - ) -} - -func getFilterCondForSeparateTable(table string, cond builder.Cond) builder.Cond { - return builder.In( - "tasks.id", - builder. - Select("task_id"). - From(table). - Where(cond), - ) -} - func getTaskIndexFromSearchString(s string) (index int64) { re := regexp.MustCompile("#([0-9]+)") in := re.FindString(s)