From 8d5a2685c4745f1a5ce1aa3b55f2df0942bee71e Mon Sep 17 00:00:00 2001 From: konrad Date: Fri, 20 Sep 2019 15:52:09 +0000 Subject: [PATCH] Fixed labels being displayed multiple times if they were associated with more than one task (#99) --- pkg/models/fixtures/label_task.yml | 6 +++++- pkg/models/label.go | 9 +++++---- pkg/models/label_task.go | 24 +++++++++++++++++------- pkg/models/task_readall_test.go | 14 ++++++++++++-- pkg/swagger/docs.go | 6 +----- pkg/swagger/swagger.json | 4 ---- pkg/swagger/swagger.yaml | 3 --- 7 files changed, 40 insertions(+), 26 deletions(-) diff --git a/pkg/models/fixtures/label_task.yml b/pkg/models/fixtures/label_task.yml index 70a9272ba..c3fd91f7b 100644 --- a/pkg/models/fixtures/label_task.yml +++ b/pkg/models/fixtures/label_task.yml @@ -1,4 +1,8 @@ - id: 1 task_id: 1 label_id: 4 - created: 0 \ No newline at end of file + created: 0 +- id: 2 + task_id: 2 + label_id: 4 + created: 0 diff --git a/pkg/models/label.go b/pkg/models/label.go index eb2cb7afe..b4bf21611 100644 --- a/pkg/models/label.go +++ b/pkg/models/label.go @@ -144,10 +144,11 @@ func (l *Label) ReadAll(search string, a web.Auth, page int) (ls interface{}, er } return getLabelsByTaskIDs(&LabelByTaskIDsOptions{ - Search: search, - User: u, - TaskIDs: taskIDs, - GetUnusedLabels: true, + Search: search, + User: u, + TaskIDs: taskIDs, + GetUnusedLabels: true, + GroupByLabelIDsOnly: true, }) } diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index 0c8f94126..707e5e0f2 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -128,17 +128,18 @@ func (lt *LabelTask) ReadAll(search string, a web.Auth, page int) (labels interf // Helper struct, contains the label + its task ID type labelWithTaskID struct { - TaskID int64 + TaskID int64 `json:"-"` Label `xorm:"extends"` } // LabelByTaskIDsOptions is a struct to not clutter the function with too many optional parameters. type LabelByTaskIDsOptions struct { - User *User - Search string - Page int - TaskIDs []int64 - GetUnusedLabels bool + User *User + Search string + Page int + TaskIDs []int64 + GetUnusedLabels bool + GroupByLabelIDsOnly bool } // Helper function to get all labels for a set of tasks @@ -153,6 +154,15 @@ func getLabelsByTaskIDs(opts *LabelByTaskIDsOptions) (ls []*labelWithTaskID, err requestOrNil = "label_task.label_id != null OR labels.created_by_id = ?" } + // We still need the task ID when we want to get all labels for a task, but because of this, we get the same label + // multiple times when it is associated to more than one task. + // Because of this whole thing, we need this extra switch here to only group by Task IDs if needed. + // Probably not the most ideal solution. + var groupBy = "labels.id,label_task.task_id" + if opts.GroupByLabelIDsOnly { + groupBy = "labels.id" + } + // Get all labels associated with these tasks var labels []*labelWithTaskID err = x.Table("labels"). @@ -161,7 +171,7 @@ func getLabelsByTaskIDs(opts *LabelByTaskIDsOptions) (ls []*labelWithTaskID, err Where(requestOrNil, uidOrNil). Or(builder.In("label_task.task_id", opts.TaskIDs)). And("labels.title LIKE ?", "%"+opts.Search+"%"). - GroupBy("labels.id,label_task.task_id"). // This filters out doubles + GroupBy(groupBy). Limit(getLimitFromPageIndex(opts.Page)). Find(&labels) if err != nil { diff --git a/pkg/models/task_readall_test.go b/pkg/models/task_readall_test.go index 0190faa94..b5646c77d 100644 --- a/pkg/models/task_readall_test.go +++ b/pkg/models/task_readall_test.go @@ -65,8 +65,18 @@ func sortTasksForTesting(by SortBy) (tasks []*Task) { CreatedByID: 1, CreatedBy: user1, ListID: 1, - Created: 1543626724, - Updated: 1543626724, + Labels: []*Label{ + { + ID: 4, + Title: "Label #4 - visible via other task", + CreatedByID: 2, + CreatedBy: user2, + Updated: 0, + Created: 0, + }, + }, + Created: 1543626724, + Updated: 1543626724, }, { ID: 3, diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 6ed359a30..8e89b147f 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -1,6 +1,6 @@ // GENERATED BY THE COMMAND ABOVE; DO NOT EDIT // This file was generated by swaggo/swag at -// 2019-08-31 22:48:49.201391811 +0200 CEST m=+0.228973511 +// 2019-09-20 17:41:39.321673846 +0200 CEST m=+0.137010842 package swagger @@ -4278,10 +4278,6 @@ var doc = `{ "description": "The ID of the shared thing", "type": "integer" }, - "list": { - "type": "object", - "$ref": "#/definitions/models.List" - }, "right": { "description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.", "type": "integer", diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index ab13676d2..8cd735108 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -4264,10 +4264,6 @@ "description": "The ID of the shared thing", "type": "integer" }, - "list": { - "type": "object", - "$ref": "#/definitions/models.List" - }, "right": { "description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.", "type": "integer", diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index df06020f8..475bcdc10 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -182,9 +182,6 @@ definitions: id: description: The ID of the shared thing type: integer - list: - $ref: '#/definitions/models.List' - type: object right: default: 0 description: The right this list is shared with. 0 = Read only, 1 = Read &