From d0606eadea06669326f9f39747d2fc49191c2e69 Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 23 Mar 2026 13:38:53 +0100 Subject: [PATCH] fix: check child project's own IsArchived flag in CheckIsArchived CheckIsArchived() previously skipped checking a child project's own IsArchived flag when ParentProjectID > 0, immediately recursing to only check the parent. This allowed write operations on individually archived child projects whose parent was not archived. Now the function loads the project from the database first, checks its own IsArchived flag, and only then recurses to check parent projects. --- pkg/db/fixtures/projects.yml | 13 +++++++++ pkg/models/project.go | 17 +++++++----- pkg/models/project_test.go | 52 +++++++++++++++++++++++++++++++++--- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/pkg/db/fixtures/projects.yml b/pkg/db/fixtures/projects.yml index 93efb6ad4..65809edff 100644 --- a/pkg/db/fixtures/projects.yml +++ b/pkg/db/fixtures/projects.yml @@ -348,3 +348,16 @@ position: 39 updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 +# Child project that is archived individually, but its parent (project 1) is NOT archived. +# Used by TestArchived to verify that CheckIsArchived checks the child's own flag. +- + id: 40 + title: Test40 child archived individually + description: Lorem Ipsum + identifier: test40 + owner_id: 1 + parent_project_id: 3 + is_archived: 1 + position: 40 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 diff --git a/pkg/models/project.go b/pkg/models/project.go index 64f46148c..cb509a1a0 100644 --- a/pkg/models/project.go +++ b/pkg/models/project.go @@ -797,12 +797,12 @@ func addMaxPermissionToProjects(s *xorm.Session, projects []*Project, u *user.Us // CheckIsArchived returns an ErrProjectIsArchived if the project or any of its parent projects is archived. func (p *Project) CheckIsArchived(s *xorm.Session) (err error) { - if p.ParentProjectID > 0 { - p := &Project{ID: p.ParentProjectID} - return p.CheckIsArchived(s) - } - - if p.ID == 0 { // don't check new projects + if p.ID == 0 { + // New project — skip checking the project itself but still check the parent. + if p.ParentProjectID > 0 { + parent := &Project{ID: p.ParentProjectID} + return parent.CheckIsArchived(s) + } return nil } @@ -815,6 +815,11 @@ func (p *Project) CheckIsArchived(s *xorm.Session) (err error) { return ErrProjectIsArchived{ProjectID: p.ID} } + if project.ParentProjectID > 0 { + parent := &Project{ID: project.ParentProjectID} + return parent.CheckIsArchived(s) + } + return nil } diff --git a/pkg/models/project_test.go b/pkg/models/project_test.go index 0c30bdb6e..5bbd124d7 100644 --- a/pkg/models/project_test.go +++ b/pkg/models/project_test.go @@ -504,12 +504,12 @@ func TestProject_ReadAll(t *testing.T) { require.NoError(t, err) assert.Equal(t, reflect.Slice, reflect.TypeOf(projects3).Kind()) ls := projects3.([]*Project) - assert.Len(t, ls, 28) + assert.Len(t, ls, 29) assert.Equal(t, int64(3), ls[0].ID) // Project 3 has a position of 1 and should be sorted first assert.Equal(t, int64(1), ls[1].ID) assert.Equal(t, int64(6), ls[2].ID) - assert.Equal(t, int64(-1), ls[26].ID) - assert.Equal(t, int64(-2), ls[27].ID) + assert.Equal(t, int64(-1), ls[27].ID) + assert.Equal(t, int64(-2), ls[28].ID) }) t.Run("projects for nonexistent user", func(t *testing.T) { db.LoadAndAssertFixtures(t) @@ -594,3 +594,49 @@ func TestProject_ReadOne(t *testing.T) { assert.NotNil(t, l.Subscription) }) } + +func TestCheckIsArchived(t *testing.T) { + t.Run("child project archived individually with non-archived parent", func(t *testing.T) { + // Project 40 is archived individually (is_archived=true) but its parent + // (project 1) is not archived. CheckIsArchived must still return an error. + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + p := &Project{ID: 40, ParentProjectID: 3} + err := p.CheckIsArchived(s) + require.Error(t, err) + assert.True(t, IsErrProjectIsArchived(err)) + }) + t.Run("root project archived", func(t *testing.T) { + // Project 22 is archived individually with no parent. + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + p := &Project{ID: 22} + err := p.CheckIsArchived(s) + require.Error(t, err) + assert.True(t, IsErrProjectIsArchived(err)) + }) + t.Run("child project inherits archived from parent", func(t *testing.T) { + // Project 21's parent (project 22) is archived. + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + p := &Project{ID: 21, ParentProjectID: 22} + err := p.CheckIsArchived(s) + require.Error(t, err) + assert.True(t, IsErrProjectIsArchived(err)) + }) + t.Run("non-archived project", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + p := &Project{ID: 1} + err := p.CheckIsArchived(s) + require.NoError(t, err) + }) +}