From 36819a8cca696814e9a005062791c1263f724d16 Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 4 Jan 2023 18:29:22 +0100 Subject: [PATCH] feat(subscriptions): make sure all subscriptions are inherited properly --- pkg/db/fixtures/projects.yml | 14 +++ pkg/db/fixtures/tasks.yml | 6 ++ pkg/models/prject.go | 27 +++++- pkg/models/subscription.go | 163 ++++++++++++++++++++++++++------ pkg/models/subscription_test.go | 23 ++++- 5 files changed, 194 insertions(+), 39 deletions(-) diff --git a/pkg/db/fixtures/projects.yml b/pkg/db/fixtures/projects.yml index 087cbdadd..329dcb647 100644 --- a/pkg/db/fixtures/projects.yml +++ b/pkg/db/fixtures/projects.yml @@ -218,3 +218,17 @@ position: 7 updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 +- + id: 25 + title: Test25 + owner_id: 6 + parent_project_id: 12 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 +- + id: 26 + title: Test26 + owner_id: 6 + parent_project_id: 25 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 diff --git a/pkg/db/fixtures/tasks.yml b/pkg/db/fixtures/tasks.yml index 4f287b989..c7173baea 100644 --- a/pkg/db/fixtures/tasks.yml +++ b/pkg/db/fixtures/tasks.yml @@ -355,3 +355,9 @@ created: 2018-12-01 01:12:04 updated: 2018-12-01 01:12:04 due_date: 2018-10-30 22:25:24 +- id: 39 + title: 'task #39' + created_by_id: 1 + project_id: 25 + created: 2018-12-01 01:12:04 + updated: 2018-12-01 01:12:04 diff --git a/pkg/models/prject.go b/pkg/models/prject.go index 02a27a747..9483d8624 100644 --- a/pkg/models/prject.go +++ b/pkg/models/prject.go @@ -46,8 +46,9 @@ type Project struct { // The hex color of this project HexColor string `xorm:"varchar(6) null" json:"hex_color" valid:"runelength(0|6)" maxLength:"6"` - OwnerID int64 `xorm:"bigint INDEX not null" json:"-"` - ParentProjectID int64 `xorm:"bigint INDEX null" json:"parent_project_id"` + OwnerID int64 `xorm:"bigint INDEX not null" json:"-"` + ParentProjectID int64 `xorm:"bigint INDEX null" json:"parent_project_id"` + ParentProject *Project `xorm:"-" json:"-"` ChildProjects []*Project `xorm:"-" json:"child_projects"` @@ -513,6 +514,22 @@ func getSavedFilterProjects(s *xorm.Session, doer *user.User) (savedFiltersProje return } +// GetAllParentProjects returns all parents of a given project +func (p *Project) GetAllParentProjects(s *xorm.Session) (err error) { + if p.ParentProjectID == 0 { + return + } + + parent, err := GetProjectSimpleByID(s, p.ParentProjectID) + if err != nil { + return err + } + + p.ParentProject = parent + + return parent.GetAllParentProjects(s) +} + // addProjectDetails adds owner user objects and project tasks to all projects in the slice func addProjectDetails(s *xorm.Session, projects []*Project, a web.Auth) (err error) { if len(projects) == 0 { @@ -541,7 +558,7 @@ func addProjectDetails(s *xorm.Session, projects []*Project, a web.Auth) (err er subscriptions, err := GetSubscriptions(s, SubscriptionEntityProject, projectIDs, a) if err != nil { log.Errorf("An error occurred while getting project subscriptions for a project: %s", err.Error()) - subscriptions = make(map[int64]*Subscription) + subscriptions = make(map[int64][]*Subscription) } for _, p := range projects { @@ -558,8 +575,8 @@ func addProjectDetails(s *xorm.Session, projects []*Project, a web.Auth) (err er } p.IsFavorite = favs[p.ID] - if subscription, exists := subscriptions[p.ID]; exists { - p.Subscription = subscription + if subscription, exists := subscriptions[p.ID]; exists && len(subscription) > 0 { + p.Subscription = subscription[0] } } diff --git a/pkg/models/subscription.go b/pkg/models/subscription.go index e646161c3..47b06babe 100644 --- a/pkg/models/subscription.go +++ b/pkg/models/subscription.go @@ -163,26 +163,26 @@ func (sb *Subscription) Delete(s *xorm.Session, auth web.Auth) (err error) { return } -func getSubscriberCondForEntity(entityType SubscriptionEntityType, entityID int64) (cond builder.Cond) { +func getSubscriberCondForEntities(entityType SubscriptionEntityType, entityIDs []int64) (cond builder.Cond) { if entityType == SubscriptionEntityProject { return builder.And( - builder.Eq{"entity_id": entityID}, + builder.In("entity_id", entityIDs), builder.Eq{"entity_type": SubscriptionEntityProject}, ) - // TODO: parent? } if entityType == SubscriptionEntityTask { return builder.Or( builder.And( - builder.Eq{"entity_id": entityID}, + builder.In("entity_id", entityIDs), builder.Eq{"entity_type": SubscriptionEntityTask}, ), builder.And( builder.Eq{"entity_id": builder. Select("project_id"). From("tasks"). - Where(builder.Eq{"id": entityID}), + Where(builder.In("id", entityIDs)), + // TODO parent project }, builder.Eq{"entity_type": SubscriptionEntityProject}, ), @@ -195,73 +195,178 @@ func getSubscriberCondForEntity(entityType SubscriptionEntityType, entityID int6 // GetSubscription returns a matching subscription for an entity and user. // It will return the next parent of a subscription. That means for tasks, it will first look for a subscription for // that task, if there is none it will look for a subscription on the project the task belongs to. -// TODO: check parent projects func GetSubscription(s *xorm.Session, entityType SubscriptionEntityType, entityID int64, a web.Auth) (subscription *Subscription, err error) { subs, err := GetSubscriptions(s, entityType, []int64{entityID}, a) if err != nil || len(subs) == 0 { return nil, err } - if sub, exists := subs[entityID]; exists { - return sub, nil // Take exact match first, if available + if sub, exists := subs[entityID]; exists && len(sub) > 0 { + return sub[0], nil // Take exact match first, if available } for _, sub := range subs { - return sub, nil // For parents, take next available + if len(sub) > 0 { + return sub[0], nil // For parents, take next available + } } return nil, nil } // GetSubscriptions returns a map of subscriptions to a set of given entity IDs -func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entityIDs []int64, a web.Auth) (projectsToSubscriptions map[int64]*Subscription, err error) { +func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entityIDs []int64, a web.Auth) (projectsToSubscriptions map[int64][]*Subscription, err error) { u, is := a.(*user.User) - if !is { + if u != nil && !is { return } if err := entityType.validate(); err != nil { return nil, err } - var entitiesFilter builder.Cond - for _, eID := range entityIDs { - if entitiesFilter == nil { - entitiesFilter = getSubscriberCondForEntity(entityType, eID) - continue + switch entityType { + case SubscriptionEntityProject: + return getSubscriptionsForProjects(s, entityIDs, u) + case SubscriptionEntityTask: + subs, err := getSubscriptionsForTasks(s, entityIDs, u) + if err != nil { + return nil, err } - entitiesFilter = entitiesFilter.Or(getSubscriberCondForEntity(entityType, eID)) + + // If the task does not have a subscription directly or from its project, get the one + // from the parent and return it instead. + for _, eID := range entityIDs { + if _, has := subs[eID]; has { + continue + } + + task, err := GetTaskByIDSimple(s, eID) + if err != nil { + return nil, err + } + projectSubscriptions, err := getSubscriptionsForProjects(s, []int64{task.ProjectID}, u) + if err != nil { + return nil, err + } + for _, subscription := range projectSubscriptions { + subs[eID] = subscription // The first project subscription is the subscription we're looking for + break + } + } + + return subs, nil + } + + return +} + +func getSubscriptionsForProjects(s *xorm.Session, projectIDs []int64, u *user.User) (projectsToSubscriptions map[int64][]*Subscription, err error) { + origEntityIDs := projectIDs + var ps = make(map[int64]*Project) + + for _, eID := range projectIDs { + ps[eID], err = GetProjectSimpleByID(s, eID) + if err != nil { + return nil, err + } + err = ps[eID].GetAllParentProjects(s) + if err != nil { + return nil, err + } + + parentIDs := []int64{} + var parent = ps[eID].ParentProject + for parent != nil { + parentIDs = append(parentIDs, parent.ID) + parent = parent.ParentProject + } + + // Now we have all parent ids + projectIDs = append(projectIDs, parentIDs...) // the child project id is already in there } var subscriptions []*Subscription - err = s. - Where("user_id = ?", u.ID). - And(entitiesFilter). - Find(&subscriptions) + if u != nil { + err = s. + Where("user_id = ?", u.ID). + And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)). + Find(&subscriptions) + } else { + err = s. + And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)). + Find(&subscriptions) + } if err != nil { return nil, err } - projectsToSubscriptions = make(map[int64]*Subscription) + projectsToSubscriptions = make(map[int64][]*Subscription) for _, sub := range subscriptions { sub.Entity = sub.EntityType.String() - projectsToSubscriptions[sub.EntityID] = sub + projectsToSubscriptions[sub.EntityID] = append(projectsToSubscriptions[sub.EntityID], sub) } + + // Rearrange so that subscriptions trickle down + + for _, eID := range origEntityIDs { + // If the current project does not have a subscription, climb up the tree until a project has one, + // then use that subscription for all child projects + _, has := projectsToSubscriptions[eID] + if !has { + var parent = ps[eID].ParentProject + for parent != nil { + sub, has := projectsToSubscriptions[parent.ID] + projectsToSubscriptions[eID] = sub + parent = parent.ParentProject + if has { // reached the top of the tree + break + } + } + } + } + return projectsToSubscriptions, nil } +func getSubscriptionsForTasks(s *xorm.Session, taskIDs []int64, u *user.User) (projectsToSubscriptions map[int64][]*Subscription, err error) { + var subscriptions []*Subscription + if u != nil { + err = s. + Where("user_id = ?", u.ID). + And(getSubscriberCondForEntities(SubscriptionEntityTask, taskIDs)). + Find(&subscriptions) + } else { + err = s. + And(getSubscriberCondForEntities(SubscriptionEntityTask, taskIDs)). + Find(&subscriptions) + } + if err != nil { + return nil, err + } + + projectsToSubscriptions = make(map[int64][]*Subscription) + for _, sub := range subscriptions { + sub.Entity = sub.EntityType.String() + projectsToSubscriptions[sub.EntityID] = append(projectsToSubscriptions[sub.EntityID], sub) + } + + return +} + func getSubscribersForEntity(s *xorm.Session, entityType SubscriptionEntityType, entityID int64) (subscriptions []*Subscription, err error) { if err := entityType.validate(); err != nil { return nil, err } - cond := getSubscriberCondForEntity(entityType, entityID) - err = s. - Where(cond). - Find(&subscriptions) + subs, err := GetSubscriptions(s, entityType, []int64{entityID}, nil) if err != nil { return } userIDs := []int64{} - for _, subscription := range subscriptions { - userIDs = append(userIDs, subscription.UserID) + subscriptions = make([]*Subscription, len(subs)) + for _, subss := range subs { + for _, subscription := range subss { + userIDs = append(userIDs, subscription.UserID) + subscriptions = append(subscriptions, subscription) + } } users, err := user.GetUsersByIDs(s, userIDs) diff --git a/pkg/models/subscription_test.go b/pkg/models/subscription_test.go index dfe4ee4f0..8dd02d07c 100644 --- a/pkg/models/subscription_test.go +++ b/pkg/models/subscription_test.go @@ -260,19 +260,32 @@ func TestSubscriptionGet(t *testing.T) { s := db.NewSession() defer s.Close() - // Project 6 belongs to project 6 where user 6 has subscribed to - sub, err := GetSubscription(s, SubscriptionEntityProject, 6, u) + // Project 25 belongs to project 12 where user 6 has subscribed to + sub, err := GetSubscription(s, SubscriptionEntityProject, 25, u) assert.NoError(t, err) assert.NotNil(t, sub) - // assert.Equal(t, int64(2), sub.ID) // TODO + assert.Equal(t, int64(12), sub.EntityID) + assert.Equal(t, int64(3), sub.ID) + }) + t.Run("project from parent's parent", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Project 26 belongs to project 25 which belongs to project 12 where user 6 has subscribed to + sub, err := GetSubscription(s, SubscriptionEntityProject, 26, u) + assert.NoError(t, err) + assert.NotNil(t, sub) + assert.Equal(t, int64(12), sub.EntityID) + assert.Equal(t, int64(3), sub.ID) }) t.Run("task from parent", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() defer s.Close() - // Task 20 belongs to project 11 which belongs to project 6 where the user has subscribed - sub, err := GetSubscription(s, SubscriptionEntityTask, 20, u) + // Task 39 belongs to project 25 which belongs to project 12 where the user has subscribed + sub, err := GetSubscription(s, SubscriptionEntityTask, 39, u) assert.NoError(t, err) assert.NotNil(t, sub) // assert.Equal(t, int64(2), sub.ID) TODO