From 9d4f0671f3749acd2596192fa1e4f886624cf9d2 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Mon, 27 May 2024 00:26:27 +0000 Subject: [PATCH 01/16] [skip ci] Updated licenses and gitignores (cherry picked from commit e625813aa9f585718e9c7677fc441f1f3ad69c61) --- ...e-first-lines => BSD-2-Clause-first-lines} | 0 options/license/Gutmann | 2 + options/license/HPND-export2-US | 21 ++++++ options/license/HPND-merchantability-variant | 9 +++ options/license/RRDtool-FLOSS-exception-2.0 | 66 +++++++++++++++++++ 5 files changed, 98 insertions(+) rename options/license/{BSD-2-clause-first-lines => BSD-2-Clause-first-lines} (100%) create mode 100644 options/license/Gutmann create mode 100644 options/license/HPND-export2-US create mode 100644 options/license/HPND-merchantability-variant create mode 100644 options/license/RRDtool-FLOSS-exception-2.0 diff --git a/options/license/BSD-2-clause-first-lines b/options/license/BSD-2-Clause-first-lines similarity index 100% rename from options/license/BSD-2-clause-first-lines rename to options/license/BSD-2-Clause-first-lines diff --git a/options/license/Gutmann b/options/license/Gutmann new file mode 100644 index 0000000000..c33f4ee3a2 --- /dev/null +++ b/options/license/Gutmann @@ -0,0 +1,2 @@ +You can use this code in whatever way you want, as long as you don't try +to claim you wrote it. diff --git a/options/license/HPND-export2-US b/options/license/HPND-export2-US new file mode 100644 index 0000000000..1dda23a88c --- /dev/null +++ b/options/license/HPND-export2-US @@ -0,0 +1,21 @@ +Copyright 2004-2008 Apple Inc. All Rights Reserved. + + Export of this software from the United States of America may + require a specific license from the United States Government. + It is the responsibility of any person or organization + contemplating export to obtain such a license before exporting. + +WITHIN THAT CONSTRAINT, permission to use, copy, modify, and +distribute this software and its documentation for any purpose and +without fee is hereby granted, provided that the above copyright +notice appear in all copies and that both that copyright notice and +this permission notice appear in supporting documentation, and that +the name of Apple Inc. not be used in advertising or publicity +pertaining to distribution of the software without specific, +written prior permission. Apple Inc. makes no representations +about the suitability of this software for any purpose. It is +provided "as is" without express or implied warranty. + +THIS SOFTWARE IS PROVIDED "AS IS" AND WITHOUT ANY EXPRESS OR +IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED +WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE. diff --git a/options/license/HPND-merchantability-variant b/options/license/HPND-merchantability-variant new file mode 100644 index 0000000000..421b9ff96b --- /dev/null +++ b/options/license/HPND-merchantability-variant @@ -0,0 +1,9 @@ +Copyright (C) 2004 Christian Groessler + +Permission to use, copy, modify, and distribute this file +for any purpose is hereby granted without fee, provided that +the above copyright notice and this notice appears in all +copies. + +This file is distributed WITHOUT ANY WARRANTY; without even the implied +warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. diff --git a/options/license/RRDtool-FLOSS-exception-2.0 b/options/license/RRDtool-FLOSS-exception-2.0 new file mode 100644 index 0000000000..d88dae5868 --- /dev/null +++ b/options/license/RRDtool-FLOSS-exception-2.0 @@ -0,0 +1,66 @@ +FLOSS License Exception +======================= +(Adapted from http://www.mysql.com/company/legal/licensing/foss-exception.html) + +I want specified Free/Libre and Open Source Software ("FLOSS") +applications to be able to use specified GPL-licensed RRDtool +libraries (the "Program") despite the fact that not all FLOSS licenses are +compatible with version 2 of the GNU General Public License (the "GPL"). + +As a special exception to the terms and conditions of version 2.0 of the GPL: + +You are free to distribute a Derivative Work that is formed entirely from +the Program and one or more works (each, a "FLOSS Work") licensed under one +or more of the licenses listed below, as long as: + +1. You obey the GPL in all respects for the Program and the Derivative +Work, except for identifiable sections of the Derivative Work which are +not derived from the Program, and which can reasonably be considered +independent and separate works in themselves, + +2. all identifiable sections of the Derivative Work which are not derived +from the Program, and which can reasonably be considered independent and +separate works in themselves, + +1. are distributed subject to one of the FLOSS licenses listed +below, and + +2. the object code or executable form of those sections are +accompanied by the complete corresponding machine-readable source +code for those sections on the same medium and under the same FLOSS +license as the corresponding object code or executable forms of +those sections, and + +3. any works which are aggregated with the Program or with a Derivative +Work on a volume of a storage or distribution medium in accordance with +the GPL, can reasonably be considered independent and separate works in +themselves which are not derivatives of either the Program, a Derivative +Work or a FLOSS Work. + +If the above conditions are not met, then the Program may only be copied, +modified, distributed or used under the terms and conditions of the GPL. + +FLOSS License List +================== +License name Version(s)/Copyright Date +Academic Free License 2.0 +Apache Software License 1.0/1.1/2.0 +Apple Public Source License 2.0 +Artistic license From Perl 5.8.0 +BSD license "July 22 1999" +Common Public License 1.0 +GNU Library or "Lesser" General Public License (LGPL) 2.0/2.1 +IBM Public License, Version 1.0 +Jabber Open Source License 1.0 +MIT License (As listed in file MIT-License.txt) - +Mozilla Public License (MPL) 1.0/1.1 +Open Software License 2.0 +OpenSSL license (with original SSLeay license) "2003" ("1998") +PHP License 3.01 +Python license (CNRI Python License) - +Python Software Foundation License 2.1.1 +Sleepycat License "1999" +W3C License "2001" +X11 License "2001" +Zlib/libpng License - +Zope Public License 2.0/2.1 From 9e495f700ef58b6a7c5560429fe317d1c4bf2f81 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 27 May 2024 13:21:00 +0800 Subject: [PATCH 02/16] Fix possible ui 500 if workflow's job is nil (#31092) Fix #31087 (cherry picked from commit e695ba47557ed4c3999c63b28051a449ca4653de) --- options/locale/locale_en-US.ini | 1 + routers/web/repo/actions/actions.go | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 721ad1605e..3f3b7143ad 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3724,6 +3724,7 @@ runs.workflow = Workflow runs.invalid_workflow_helper = Workflow config file is invalid. Please check your config file: %s runs.no_matching_online_runner_helper = No matching online runner with label: %s runs.no_job_without_needs = The workflow must contain at least one job without dependencies. +runs.no_job = The workflow must contain at least one job runs.actor = Actor runs.status = Status runs.actors_no_select = All actors diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 6059ad1414..a0f03ec7e9 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -107,7 +107,12 @@ func List(ctx *context.Context) { // The workflow must contain at least one job without "needs". Otherwise, a deadlock will occur and no jobs will be able to run. hasJobWithoutNeeds := false // Check whether have matching runner and a job without "needs" + emptyJobsNumber := 0 for _, j := range wf.Jobs { + if j == nil { + emptyJobsNumber++ + continue + } if !hasJobWithoutNeeds && len(j.Needs()) == 0 { hasJobWithoutNeeds = true } @@ -131,6 +136,9 @@ func List(ctx *context.Context) { if !hasJobWithoutNeeds { workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs") } + if emptyJobsNumber == len(wf.Jobs) { + workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job") + } workflows = append(workflows, workflow) } } From ec05ab1e3c99270c46e93fc3113d6b2102a14c57 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Mon, 27 May 2024 14:15:34 +0800 Subject: [PATCH 03/16] Improve the handling of `jobs..if` (#31070) Fix #25897 Fix #30322 #29464 cannot handle some complex `if` conditions correctly because it only checks `always()` literally. In fact, it's not easy to evaluate the `if` condition on the Gitea side because evaluating it requires a series of contexts. But act_runner is able to evaluate the `if` condition before running the job (for more information, see [`gitea/act`](https://gitea.com/gitea/act/src/commit/517d11c67126bd97c88e2faabda0832fff482258/pkg/runner/run_context.go#L739-L753)) . So we can use act_runner to check the `if` condition. In this PR, how to handle a blocked job depends on its `needs` and `if`: - If not all jobs in `needs` completed successfully and the job's `if` is empty, set the job status to `StatusSkipped` - In other cases, the job status will be set to `StatusWaiting`, and then act_runner will check the `if` condition and run the job if the condition is met (cherry picked from commit 31a0c4dfb4156a7b4d856cceae1e61c7fc1a4a1b) --- services/actions/job_emitter.go | 14 +++++++------- services/actions/job_emitter_test.go | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index d2bbbd9a7c..1f859fcf70 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -7,7 +7,6 @@ import ( "context" "errors" "fmt" - "strings" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" @@ -141,18 +140,19 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { if allSucceed { ret[id] = actions_model.StatusWaiting } else { - // If a job's "if" condition is "always()", the job should always run even if some of its dependencies did not succeed. - // See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds - always := false + // Check if the job has an "if" condition + hasIf := false if wfJobs, _ := jobparser.Parse(r.jobMap[id].WorkflowPayload); len(wfJobs) == 1 { _, wfJob := wfJobs[0].Job() - expr := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(wfJob.If.Value, "${{"), "}}")) - always = expr == "always()" + hasIf = len(wfJob.If.Value) > 0 } - if always { + if hasIf { + // act_runner will check the "if" condition ret[id] = actions_model.StatusWaiting } else { + // If the "if" condition is empty and not all dependent jobs completed successfully, + // the job should be skipped. ret[id] = actions_model.StatusSkipped } } diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 038df7d4f8..58c2dc3b24 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -71,9 +71,9 @@ func Test_jobStatusResolver_Resolve(t *testing.T) { want: map[int64]actions_model.Status{}, }, { - name: "with ${{ always() }} condition", + name: "`if` is not empty and all jobs in `needs` completed successfully", jobs: actions_model.ActionJobList{ - {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, + {ID: 1, JobID: "job1", Status: actions_model.StatusSuccess, Needs: []string{}}, {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( ` name: test @@ -82,15 +82,15 @@ jobs: job2: runs-on: ubuntu-latest needs: job1 - if: ${{ always() }} + if: ${{ always() && needs.job1.result == 'success' }} steps: - - run: echo "always run" + - run: echo "will be checked by act_runner" `)}, }, want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, }, { - name: "with always() condition", + name: "`if` is not empty and not all jobs in `needs` completed successfully", jobs: actions_model.ActionJobList{ {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( @@ -101,15 +101,15 @@ jobs: job2: runs-on: ubuntu-latest needs: job1 - if: always() + if: ${{ always() && needs.job1.result == 'failure' }} steps: - - run: echo "always run" + - run: echo "will be checked by act_runner" `)}, }, want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, }, { - name: "without always() condition", + name: "`if` is empty and not all jobs in `needs` completed successfully", jobs: actions_model.ActionJobList{ {ID: 1, JobID: "job1", Status: actions_model.StatusFailure, Needs: []string{}}, {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( @@ -121,7 +121,7 @@ jobs: runs-on: ubuntu-latest needs: job1 steps: - - run: echo "not always run" + - run: echo "should be skipped" `)}, }, want: map[int64]actions_model.Status{2: actions_model.StatusSkipped}, From a7591f9738dbefb2dcddeb2d45175abee3d03c1f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 27 May 2024 16:59:54 +0800 Subject: [PATCH 04/16] Rename project board -> column to make the UI less confusing (#30170) This PR split the `Board` into two parts. One is the struct has been renamed to `Column` and the second we have a `Template Type`. But to make it easier to review, this PR will not change the database schemas, they are just renames. The database schema changes could be in future PRs. --------- Co-authored-by: silverwind Co-authored-by: yp05327 <576951401@qq.com> (cherry picked from commit 98751108b11dc748cc99230ca0fc1acfdf2c8929) Conflicts: docs/content/administration/config-cheat-sheet.en-us.md docs/content/index.en-us.md docs/content/installation/comparison.en-us.md docs/content/usage/permissions.en-us.md non existent files options/locale/locale_en-US.ini routers/web/web.go templates/repo/header.tmpl templates/repo/settings/options.tmpl trivial context conflicts --- models/activities/statistic.go | 4 +- models/issues/comment.go | 6 +- models/issues/issue_project.go | 38 +- models/issues/issue_search.go | 14 +- models/migrations/v1_22/v293_test.go | 24 +- models/project/board.go | 389 ------------------ models/project/column.go | 359 ++++++++++++++++ .../project/{board_test.go => column_test.go} | 48 +-- models/project/issue.go | 24 +- models/project/project.go | 97 ++--- models/project/project_test.go | 14 +- models/project/template.go | 45 ++ models/unit/unit.go | 2 +- modules/indexer/issues/bleve/bleve.go | 4 +- modules/indexer/issues/db/options.go | 2 +- modules/indexer/issues/dboptions.go | 2 +- .../issues/elasticsearch/elasticsearch.go | 4 +- modules/indexer/issues/indexer_test.go | 4 +- modules/indexer/issues/internal/model.go | 6 +- .../indexer/issues/internal/tests/tests.go | 18 +- .../indexer/issues/meilisearch/meilisearch.go | 4 +- modules/indexer/issues/util.go | 2 +- modules/metrics/collector.go | 14 +- options/locale/locale_en-US.ini | 2 +- routers/web/org/projects.go | 114 +++-- routers/web/org/projects_test.go | 8 +- routers/web/repo/issue.go | 12 +- routers/web/repo/projects.go | 118 +++--- routers/web/repo/projects_test.go | 8 +- routers/web/web.go | 20 +- services/forms/repo_form.go | 50 +-- services/forms/user_form_hidden_comments.go | 2 +- templates/projects/new.tmpl | 6 +- templates/repo/header.tmpl | 2 +- templates/repo/issue/filter_actions.tmpl | 2 +- templates/repo/settings/units/overview.tmpl | 2 +- tests/integration/project_test.go | 14 +- web_src/css/features/projects.css | 2 +- web_src/css/themes/theme-gitea-dark.css | 2 +- web_src/css/themes/theme-gitea-light.css | 2 +- 40 files changed, 720 insertions(+), 770 deletions(-) delete mode 100644 models/project/board.go create mode 100644 models/project/column.go rename models/project/{board_test.go => column_test.go} (69%) create mode 100644 models/project/template.go diff --git a/models/activities/statistic.go b/models/activities/statistic.go index d1a459d1b2..ff81ad78a1 100644 --- a/models/activities/statistic.go +++ b/models/activities/statistic.go @@ -30,7 +30,7 @@ type Statistic struct { Mirror, Release, AuthSource, Webhook, Milestone, Label, HookTask, Team, UpdateTask, Project, - ProjectBoard, Attachment, + ProjectColumn, Attachment, Branches, Tags, CommitStatus int64 IssueByLabel []IssueByLabelCount IssueByRepository []IssueByRepositoryCount @@ -115,6 +115,6 @@ func GetStatistic(ctx context.Context) (stats Statistic) { stats.Counter.Team, _ = e.Count(new(organization.Team)) stats.Counter.Attachment, _ = e.Count(new(repo_model.Attachment)) stats.Counter.Project, _ = e.Count(new(project_model.Project)) - stats.Counter.ProjectBoard, _ = e.Count(new(project_model.Board)) + stats.Counter.ProjectColumn, _ = e.Count(new(project_model.Column)) return stats } diff --git a/models/issues/comment.go b/models/issues/comment.go index e4b5ed12cd..b0f7c47a73 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -100,8 +100,8 @@ const ( CommentTypeMergePull // 28 merge pull request CommentTypePullRequestPush // 29 push to PR head branch - CommentTypeProject // 30 Project changed - CommentTypeProjectBoard // 31 Project board changed + CommentTypeProject // 30 Project changed + CommentTypeProjectColumn // 31 Project column changed CommentTypeDismissReview // 32 Dismiss Review @@ -146,7 +146,7 @@ var commentStrings = []string{ "merge_pull", "pull_push", "project", - "project_board", + "project_board", // FIXME: the name should be project_column "dismiss_review", "change_issue_ref", "pull_scheduled_merge", diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index e31d2ef151..835ea1db52 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -37,22 +37,22 @@ func (issue *Issue) projectID(ctx context.Context) int64 { return ip.ProjectID } -// ProjectBoardID return project board id if issue was assigned to one -func (issue *Issue) ProjectBoardID(ctx context.Context) int64 { +// ProjectColumnID return project column id if issue was assigned to one +func (issue *Issue) ProjectColumnID(ctx context.Context) int64 { var ip project_model.ProjectIssue has, err := db.GetEngine(ctx).Where("issue_id=?", issue.ID).Get(&ip) if err != nil || !has { return 0 } - return ip.ProjectBoardID + return ip.ProjectColumnID } -// LoadIssuesFromBoard load issues assigned to this board -func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList, error) { +// LoadIssuesFromColumn load issues assigned to this column +func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column) (IssueList, error) { issueList, err := Issues(ctx, &IssuesOptions{ - ProjectBoardID: b.ID, - ProjectID: b.ProjectID, - SortType: "project-column-sorting", + ProjectColumnID: b.ID, + ProjectID: b.ProjectID, + SortType: "project-column-sorting", }) if err != nil { return nil, err @@ -60,9 +60,9 @@ func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList if b.Default { issues, err := Issues(ctx, &IssuesOptions{ - ProjectBoardID: db.NoConditionID, - ProjectID: b.ProjectID, - SortType: "project-column-sorting", + ProjectColumnID: db.NoConditionID, + ProjectID: b.ProjectID, + SortType: "project-column-sorting", }) if err != nil { return nil, err @@ -77,11 +77,11 @@ func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList return issueList, nil } -// LoadIssuesFromBoardList load issues assigned to the boards -func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (map[int64]IssueList, error) { +// LoadIssuesFromColumnList load issues assigned to the columns +func LoadIssuesFromColumnList(ctx context.Context, bs project_model.ColumnList) (map[int64]IssueList, error) { issuesMap := make(map[int64]IssueList, len(bs)) for i := range bs { - il, err := LoadIssuesFromBoard(ctx, bs[i]) + il, err := LoadIssuesFromColumn(ctx, bs[i]) if err != nil { return nil, err } @@ -110,7 +110,7 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) } if newColumnID == 0 { - newDefaultColumn, err := newProject.GetDefaultBoard(ctx) + newDefaultColumn, err := newProject.GetDefaultColumn(ctx) if err != nil { return err } @@ -153,10 +153,10 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo } newSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) return db.Insert(ctx, &project_model.ProjectIssue{ - IssueID: issue.ID, - ProjectID: newProjectID, - ProjectBoardID: newColumnID, - Sorting: newSorting, + IssueID: issue.ID, + ProjectID: newProjectID, + ProjectColumnID: newColumnID, + Sorting: newSorting, }) }) } diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 921dd9973e..491def1229 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -33,7 +33,7 @@ type IssuesOptions struct { //nolint SubscriberID int64 MilestoneIDs []int64 ProjectID int64 - ProjectBoardID int64 + ProjectColumnID int64 IsClosed optional.Option[bool] IsPull optional.Option[bool] LabelIDs []int64 @@ -169,12 +169,12 @@ func applyProjectCondition(sess *xorm.Session, opts *IssuesOptions) *xorm.Sessio return sess } -func applyProjectBoardCondition(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { - // opts.ProjectBoardID == 0 means all project boards, +func applyProjectColumnCondition(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { + // opts.ProjectColumnID == 0 means all project columns, // do not need to apply any condition - if opts.ProjectBoardID > 0 { - sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": opts.ProjectBoardID})) - } else if opts.ProjectBoardID == db.NoConditionID { + if opts.ProjectColumnID > 0 { + sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": opts.ProjectColumnID})) + } else if opts.ProjectColumnID == db.NoConditionID { sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": 0})) } return sess @@ -246,7 +246,7 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { applyProjectCondition(sess, opts) - applyProjectBoardCondition(sess, opts) + applyProjectColumnCondition(sess, opts) if opts.IsPull.Has() { sess.And("issue.is_pull=?", opts.IsPull.Value()) diff --git a/models/migrations/v1_22/v293_test.go b/models/migrations/v1_22/v293_test.go index ccc92f39a6..cfe4345143 100644 --- a/models/migrations/v1_22/v293_test.go +++ b/models/migrations/v1_22/v293_test.go @@ -15,7 +15,7 @@ import ( func Test_CheckProjectColumnsConsistency(t *testing.T) { // Prepare and load the testing database - x, deferable := base.PrepareTestEnv(t, 0, new(project.Project), new(project.Board)) + x, deferable := base.PrepareTestEnv(t, 0, new(project.Project), new(project.Column)) defer deferable() if x == nil || t.Failed() { return @@ -23,22 +23,22 @@ func Test_CheckProjectColumnsConsistency(t *testing.T) { assert.NoError(t, CheckProjectColumnsConsistency(x)) - // check if default board was added - var defaultBoard project.Board - has, err := x.Where("project_id=? AND `default` = ?", 1, true).Get(&defaultBoard) + // check if default column was added + var defaultColumn project.Column + has, err := x.Where("project_id=? AND `default` = ?", 1, true).Get(&defaultColumn) assert.NoError(t, err) assert.True(t, has) - assert.Equal(t, int64(1), defaultBoard.ProjectID) - assert.True(t, defaultBoard.Default) + assert.Equal(t, int64(1), defaultColumn.ProjectID) + assert.True(t, defaultColumn.Default) // check if multiple defaults, previous were removed and last will be kept - expectDefaultBoard, err := project.GetBoard(db.DefaultContext, 2) + expectDefaultColumn, err := project.GetColumn(db.DefaultContext, 2) assert.NoError(t, err) - assert.Equal(t, int64(2), expectDefaultBoard.ProjectID) - assert.False(t, expectDefaultBoard.Default) + assert.Equal(t, int64(2), expectDefaultColumn.ProjectID) + assert.False(t, expectDefaultColumn.Default) - expectNonDefaultBoard, err := project.GetBoard(db.DefaultContext, 3) + expectNonDefaultColumn, err := project.GetColumn(db.DefaultContext, 3) assert.NoError(t, err) - assert.Equal(t, int64(2), expectNonDefaultBoard.ProjectID) - assert.True(t, expectNonDefaultBoard.Default) + assert.Equal(t, int64(2), expectNonDefaultColumn.ProjectID) + assert.True(t, expectNonDefaultColumn.Default) } diff --git a/models/project/board.go b/models/project/board.go deleted file mode 100644 index a52baa0c18..0000000000 --- a/models/project/board.go +++ /dev/null @@ -1,389 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package project - -import ( - "context" - "errors" - "fmt" - "regexp" - - "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" - - "xorm.io/builder" -) - -type ( - // BoardType is used to represent a project board type - BoardType uint8 - - // CardType is used to represent a project board card type - CardType uint8 - - // BoardList is a list of all project boards in a repository - BoardList []*Board -) - -const ( - // BoardTypeNone is a project board type that has no predefined columns - BoardTypeNone BoardType = iota - - // BoardTypeBasicKanban is a project board type that has basic predefined columns - BoardTypeBasicKanban - - // BoardTypeBugTriage is a project board type that has predefined columns suited to hunting down bugs - BoardTypeBugTriage -) - -const ( - // CardTypeTextOnly is a project board card type that is text only - CardTypeTextOnly CardType = iota - - // CardTypeImagesAndText is a project board card type that has images and text - CardTypeImagesAndText -) - -// BoardColorPattern is a regexp witch can validate BoardColor -var BoardColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$") - -// Board is used to represent boards on a project -type Board struct { - ID int64 `xorm:"pk autoincr"` - Title string - Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific board will be assigned to this board - Sorting int8 `xorm:"NOT NULL DEFAULT 0"` - Color string `xorm:"VARCHAR(7)"` - - ProjectID int64 `xorm:"INDEX NOT NULL"` - CreatorID int64 `xorm:"NOT NULL"` - - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` -} - -// TableName return the real table name -func (Board) TableName() string { - return "project_board" -} - -// NumIssues return counter of all issues assigned to the board -func (b *Board) NumIssues(ctx context.Context) int { - c, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", b.ProjectID). - And("project_board_id=?", b.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() - if err != nil { - return 0 - } - return int(c) -} - -func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { - issues := make([]*ProjectIssue, 0, 5) - if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID). - And("project_board_id=?", b.ID). - OrderBy("sorting, id"). - Find(&issues); err != nil { - return nil, err - } - return issues, nil -} - -func init() { - db.RegisterModel(new(Board)) -} - -// IsBoardTypeValid checks if the project board type is valid -func IsBoardTypeValid(p BoardType) bool { - switch p { - case BoardTypeNone, BoardTypeBasicKanban, BoardTypeBugTriage: - return true - default: - return false - } -} - -// IsCardTypeValid checks if the project board card type is valid -func IsCardTypeValid(p CardType) bool { - switch p { - case CardTypeTextOnly, CardTypeImagesAndText: - return true - default: - return false - } -} - -func createBoardsForProjectsType(ctx context.Context, project *Project) error { - var items []string - - switch project.BoardType { - case BoardTypeBugTriage: - items = setting.Project.ProjectBoardBugTriageType - - case BoardTypeBasicKanban: - items = setting.Project.ProjectBoardBasicKanbanType - case BoardTypeNone: - fallthrough - default: - return nil - } - - board := Board{ - CreatedUnix: timeutil.TimeStampNow(), - CreatorID: project.CreatorID, - Title: "Backlog", - ProjectID: project.ID, - Default: true, - } - if err := db.Insert(ctx, board); err != nil { - return err - } - - if len(items) == 0 { - return nil - } - - boards := make([]Board, 0, len(items)) - - for _, v := range items { - boards = append(boards, Board{ - CreatedUnix: timeutil.TimeStampNow(), - CreatorID: project.CreatorID, - Title: v, - ProjectID: project.ID, - }) - } - - return db.Insert(ctx, boards) -} - -// maxProjectColumns max columns allowed in a project, this should not bigger than 127 -// because sorting is int8 in database -const maxProjectColumns = 20 - -// NewBoard adds a new project board to a given project -func NewBoard(ctx context.Context, board *Board) error { - if len(board.Color) != 0 && !BoardColorPattern.MatchString(board.Color) { - return fmt.Errorf("bad color code: %s", board.Color) - } - res := struct { - MaxSorting int64 - ColumnCount int64 - }{} - if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as column_count").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&res); err != nil { - return err - } - if res.ColumnCount >= maxProjectColumns { - return fmt.Errorf("NewBoard: maximum number of columns reached") - } - board.Sorting = int8(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0)) - _, err := db.GetEngine(ctx).Insert(board) - return err -} - -// DeleteBoardByID removes all issues references to the project board. -func DeleteBoardByID(ctx context.Context, boardID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := deleteBoardByID(ctx, boardID); err != nil { - return err - } - - return committer.Commit() -} - -func deleteBoardByID(ctx context.Context, boardID int64) error { - board, err := GetBoard(ctx, boardID) - if err != nil { - if IsErrProjectBoardNotExist(err) { - return nil - } - - return err - } - - if board.Default { - return fmt.Errorf("deleteBoardByID: cannot delete default board") - } - - // move all issues to the default column - project, err := GetProjectByID(ctx, board.ProjectID) - if err != nil { - return err - } - defaultColumn, err := project.GetDefaultBoard(ctx) - if err != nil { - return err - } - - if err = board.moveIssuesToAnotherColumn(ctx, defaultColumn); err != nil { - return err - } - - if _, err := db.GetEngine(ctx).ID(board.ID).NoAutoCondition().Delete(board); err != nil { - return err - } - return nil -} - -func deleteBoardByProjectID(ctx context.Context, projectID int64) error { - _, err := db.GetEngine(ctx).Where("project_id=?", projectID).Delete(&Board{}) - return err -} - -// GetBoard fetches the current board of a project -func GetBoard(ctx context.Context, boardID int64) (*Board, error) { - board := new(Board) - has, err := db.GetEngine(ctx).ID(boardID).Get(board) - if err != nil { - return nil, err - } else if !has { - return nil, ErrProjectBoardNotExist{BoardID: boardID} - } - - return board, nil -} - -// UpdateBoard updates a project board -func UpdateBoard(ctx context.Context, board *Board) error { - var fieldToUpdate []string - - if board.Sorting != 0 { - fieldToUpdate = append(fieldToUpdate, "sorting") - } - - if board.Title != "" { - fieldToUpdate = append(fieldToUpdate, "title") - } - - if len(board.Color) != 0 && !BoardColorPattern.MatchString(board.Color) { - return fmt.Errorf("bad color code: %s", board.Color) - } - fieldToUpdate = append(fieldToUpdate, "color") - - _, err := db.GetEngine(ctx).ID(board.ID).Cols(fieldToUpdate...).Update(board) - - return err -} - -// GetBoards fetches all boards related to a project -func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { - boards := make([]*Board, 0, 5) - if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Find(&boards); err != nil { - return nil, err - } - - return boards, nil -} - -// GetDefaultBoard return default board and ensure only one exists -func (p *Project) GetDefaultBoard(ctx context.Context) (*Board, error) { - var board Board - has, err := db.GetEngine(ctx). - Where("project_id=? AND `default` = ?", p.ID, true). - Desc("id").Get(&board) - if err != nil { - return nil, err - } - - if has { - return &board, nil - } - - // create a default board if none is found - board = Board{ - ProjectID: p.ID, - Default: true, - Title: "Uncategorized", - CreatorID: p.CreatorID, - } - if _, err := db.GetEngine(ctx).Insert(&board); err != nil { - return nil, err - } - return &board, nil -} - -// SetDefaultBoard represents a board for issues not assigned to one -func SetDefaultBoard(ctx context.Context, projectID, boardID int64) error { - return db.WithTx(ctx, func(ctx context.Context) error { - if _, err := GetBoard(ctx, boardID); err != nil { - return err - } - - if _, err := db.GetEngine(ctx).Where(builder.Eq{ - "project_id": projectID, - "`default`": true, - }).Cols("`default`").Update(&Board{Default: false}); err != nil { - return err - } - - _, err := db.GetEngine(ctx).ID(boardID). - Where(builder.Eq{"project_id": projectID}). - Cols("`default`").Update(&Board{Default: true}) - return err - }) -} - -// UpdateBoardSorting update project board sorting -func UpdateBoardSorting(ctx context.Context, bs BoardList) error { - return db.WithTx(ctx, func(ctx context.Context) error { - for i := range bs { - if _, err := db.GetEngine(ctx).ID(bs[i].ID).Cols( - "sorting", - ).Update(bs[i]); err != nil { - return err - } - } - return nil - }) -} - -func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (BoardList, error) { - columns := make([]*Board, 0, 5) - if err := db.GetEngine(ctx). - Where("project_id =?", projectID). - In("id", columnsIDs). - OrderBy("sorting").Find(&columns); err != nil { - return nil, err - } - return columns, nil -} - -// MoveColumnsOnProject sorts columns in a project -func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { - return db.WithTx(ctx, func(ctx context.Context) error { - sess := db.GetEngine(ctx) - columnIDs := util.ValuesOfMap(sortedColumnIDs) - movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) - if err != nil { - return err - } - if len(movedColumns) != len(sortedColumnIDs) { - return errors.New("some columns do not exist") - } - - for _, column := range movedColumns { - if column.ProjectID != project.ID { - return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) - } - } - - for sorting, columnID := range sortedColumnIDs { - if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { - return err - } - } - return nil - }) -} diff --git a/models/project/column.go b/models/project/column.go new file mode 100644 index 0000000000..222f448599 --- /dev/null +++ b/models/project/column.go @@ -0,0 +1,359 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +import ( + "context" + "errors" + "fmt" + "regexp" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" + + "xorm.io/builder" +) + +type ( + + // CardType is used to represent a project column card type + CardType uint8 + + // ColumnList is a list of all project columns in a repository + ColumnList []*Column +) + +const ( + // CardTypeTextOnly is a project column card type that is text only + CardTypeTextOnly CardType = iota + + // CardTypeImagesAndText is a project column card type that has images and text + CardTypeImagesAndText +) + +// ColumnColorPattern is a regexp witch can validate ColumnColor +var ColumnColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$") + +// Column is used to represent column on a project +type Column struct { + ID int64 `xorm:"pk autoincr"` + Title string + Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific column will be assigned to this column + Sorting int8 `xorm:"NOT NULL DEFAULT 0"` + Color string `xorm:"VARCHAR(7)"` + + ProjectID int64 `xorm:"INDEX NOT NULL"` + CreatorID int64 `xorm:"NOT NULL"` + + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` +} + +// TableName return the real table name +func (Column) TableName() string { + return "project_board" // TODO: the legacy table name should be project_column +} + +// NumIssues return counter of all issues assigned to the column +func (c *Column) NumIssues(ctx context.Context) int { + total, err := db.GetEngine(ctx).Table("project_issue"). + Where("project_id=?", c.ProjectID). + And("project_board_id=?", c.ID). + GroupBy("issue_id"). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(total) +} + +func (c *Column) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { + issues := make([]*ProjectIssue, 0, 5) + if err := db.GetEngine(ctx).Where("project_id=?", c.ProjectID). + And("project_board_id=?", c.ID). + OrderBy("sorting, id"). + Find(&issues); err != nil { + return nil, err + } + return issues, nil +} + +func init() { + db.RegisterModel(new(Column)) +} + +// IsCardTypeValid checks if the project column card type is valid +func IsCardTypeValid(p CardType) bool { + switch p { + case CardTypeTextOnly, CardTypeImagesAndText: + return true + default: + return false + } +} + +func createDefaultColumnsForProject(ctx context.Context, project *Project) error { + var items []string + + switch project.TemplateType { + case TemplateTypeBugTriage: + items = setting.Project.ProjectBoardBugTriageType + case TemplateTypeBasicKanban: + items = setting.Project.ProjectBoardBasicKanbanType + case TemplateTypeNone: + fallthrough + default: + return nil + } + + return db.WithTx(ctx, func(ctx context.Context) error { + column := Column{ + CreatedUnix: timeutil.TimeStampNow(), + CreatorID: project.CreatorID, + Title: "Backlog", + ProjectID: project.ID, + Default: true, + } + if err := db.Insert(ctx, column); err != nil { + return err + } + + if len(items) == 0 { + return nil + } + + columns := make([]Column, 0, len(items)) + for _, v := range items { + columns = append(columns, Column{ + CreatedUnix: timeutil.TimeStampNow(), + CreatorID: project.CreatorID, + Title: v, + ProjectID: project.ID, + }) + } + + return db.Insert(ctx, columns) + }) +} + +// maxProjectColumns max columns allowed in a project, this should not bigger than 127 +// because sorting is int8 in database +const maxProjectColumns = 20 + +// NewColumn adds a new project column to a given project +func NewColumn(ctx context.Context, column *Column) error { + if len(column.Color) != 0 && !ColumnColorPattern.MatchString(column.Color) { + return fmt.Errorf("bad color code: %s", column.Color) + } + + res := struct { + MaxSorting int64 + ColumnCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as column_count").Table("project_board"). + Where("project_id=?", column.ProjectID).Get(&res); err != nil { + return err + } + if res.ColumnCount >= maxProjectColumns { + return fmt.Errorf("NewBoard: maximum number of columns reached") + } + column.Sorting = int8(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0)) + _, err := db.GetEngine(ctx).Insert(column) + return err +} + +// DeleteColumnByID removes all issues references to the project column. +func DeleteColumnByID(ctx context.Context, columnID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + return deleteColumnByID(ctx, columnID) + }) +} + +func deleteColumnByID(ctx context.Context, columnID int64) error { + column, err := GetColumn(ctx, columnID) + if err != nil { + if IsErrProjectColumnNotExist(err) { + return nil + } + + return err + } + + if column.Default { + return fmt.Errorf("deleteColumnByID: cannot delete default column") + } + + // move all issues to the default column + project, err := GetProjectByID(ctx, column.ProjectID) + if err != nil { + return err + } + defaultColumn, err := project.GetDefaultColumn(ctx) + if err != nil { + return err + } + + if err = column.moveIssuesToAnotherColumn(ctx, defaultColumn); err != nil { + return err + } + + if _, err := db.GetEngine(ctx).ID(column.ID).NoAutoCondition().Delete(column); err != nil { + return err + } + return nil +} + +func deleteColumnByProjectID(ctx context.Context, projectID int64) error { + _, err := db.GetEngine(ctx).Where("project_id=?", projectID).Delete(&Column{}) + return err +} + +// GetColumn fetches the current column of a project +func GetColumn(ctx context.Context, columnID int64) (*Column, error) { + column := new(Column) + has, err := db.GetEngine(ctx).ID(columnID).Get(column) + if err != nil { + return nil, err + } else if !has { + return nil, ErrProjectColumnNotExist{ColumnID: columnID} + } + + return column, nil +} + +// UpdateColumn updates a project column +func UpdateColumn(ctx context.Context, column *Column) error { + var fieldToUpdate []string + + if column.Sorting != 0 { + fieldToUpdate = append(fieldToUpdate, "sorting") + } + + if column.Title != "" { + fieldToUpdate = append(fieldToUpdate, "title") + } + + if len(column.Color) != 0 && !ColumnColorPattern.MatchString(column.Color) { + return fmt.Errorf("bad color code: %s", column.Color) + } + fieldToUpdate = append(fieldToUpdate, "color") + + _, err := db.GetEngine(ctx).ID(column.ID).Cols(fieldToUpdate...).Update(column) + + return err +} + +// GetColumns fetches all columns related to a project +func (p *Project) GetColumns(ctx context.Context) (ColumnList, error) { + columns := make([]*Column, 0, 5) + if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Find(&columns); err != nil { + return nil, err + } + + return columns, nil +} + +// GetDefaultColumn return default column and ensure only one exists +func (p *Project) GetDefaultColumn(ctx context.Context) (*Column, error) { + var column Column + has, err := db.GetEngine(ctx). + Where("project_id=? AND `default` = ?", p.ID, true). + Desc("id").Get(&column) + if err != nil { + return nil, err + } + + if has { + return &column, nil + } + + // create a default column if none is found + column = Column{ + ProjectID: p.ID, + Default: true, + Title: "Uncategorized", + CreatorID: p.CreatorID, + } + if _, err := db.GetEngine(ctx).Insert(&column); err != nil { + return nil, err + } + return &column, nil +} + +// SetDefaultColumn represents a column for issues not assigned to one +func SetDefaultColumn(ctx context.Context, projectID, columnID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := GetColumn(ctx, columnID); err != nil { + return err + } + + if _, err := db.GetEngine(ctx).Where(builder.Eq{ + "project_id": projectID, + "`default`": true, + }).Cols("`default`").Update(&Column{Default: false}); err != nil { + return err + } + + _, err := db.GetEngine(ctx).ID(columnID). + Where(builder.Eq{"project_id": projectID}). + Cols("`default`").Update(&Column{Default: true}) + return err + }) +} + +// UpdateColumnSorting update project column sorting +func UpdateColumnSorting(ctx context.Context, cl ColumnList) error { + return db.WithTx(ctx, func(ctx context.Context) error { + for i := range cl { + if _, err := db.GetEngine(ctx).ID(cl[i].ID).Cols( + "sorting", + ).Update(cl[i]); err != nil { + return err + } + } + return nil + }) +} + +func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (ColumnList, error) { + columns := make([]*Column, 0, 5) + if err := db.GetEngine(ctx). + Where("project_id =?", projectID). + In("id", columnsIDs). + OrderBy("sorting").Find(&columns); err != nil { + return nil, err + } + return columns, nil +} + +// MoveColumnsOnProject sorts columns in a project +func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) + columnIDs := util.ValuesOfMap(sortedColumnIDs) + movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) + if err != nil { + return err + } + if len(movedColumns) != len(sortedColumnIDs) { + return errors.New("some columns do not exist") + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) + } + } + + for sorting, columnID := range sortedColumnIDs { + if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { + return err + } + } + return nil + }) +} diff --git a/models/project/board_test.go b/models/project/column_test.go similarity index 69% rename from models/project/board_test.go rename to models/project/column_test.go index da922ff7ad..911649fb72 100644 --- a/models/project/board_test.go +++ b/models/project/column_test.go @@ -14,48 +14,48 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetDefaultBoard(t *testing.T) { +func TestGetDefaultColumn(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) projectWithoutDefault, err := GetProjectByID(db.DefaultContext, 5) assert.NoError(t, err) - // check if default board was added - board, err := projectWithoutDefault.GetDefaultBoard(db.DefaultContext) + // check if default column was added + column, err := projectWithoutDefault.GetDefaultColumn(db.DefaultContext) assert.NoError(t, err) - assert.Equal(t, int64(5), board.ProjectID) - assert.Equal(t, "Uncategorized", board.Title) + assert.Equal(t, int64(5), column.ProjectID) + assert.Equal(t, "Uncategorized", column.Title) projectWithMultipleDefaults, err := GetProjectByID(db.DefaultContext, 6) assert.NoError(t, err) // check if multiple defaults were removed - board, err = projectWithMultipleDefaults.GetDefaultBoard(db.DefaultContext) + column, err = projectWithMultipleDefaults.GetDefaultColumn(db.DefaultContext) assert.NoError(t, err) - assert.Equal(t, int64(6), board.ProjectID) - assert.Equal(t, int64(9), board.ID) + assert.Equal(t, int64(6), column.ProjectID) + assert.Equal(t, int64(9), column.ID) - // set 8 as default board - assert.NoError(t, SetDefaultBoard(db.DefaultContext, board.ProjectID, 8)) + // set 8 as default column + assert.NoError(t, SetDefaultColumn(db.DefaultContext, column.ProjectID, 8)) - // then 9 will become a non-default board - board, err = GetBoard(db.DefaultContext, 9) + // then 9 will become a non-default column + column, err = GetColumn(db.DefaultContext, 9) assert.NoError(t, err) - assert.Equal(t, int64(6), board.ProjectID) - assert.False(t, board.Default) + assert.Equal(t, int64(6), column.ProjectID) + assert.False(t, column.Default) } func Test_moveIssuesToAnotherColumn(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - column1 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 1, ProjectID: 1}) + column1 := unittest.AssertExistsAndLoadBean(t, &Column{ID: 1, ProjectID: 1}) issues, err := column1.GetIssues(db.DefaultContext) assert.NoError(t, err) assert.Len(t, issues, 1) assert.EqualValues(t, 1, issues[0].ID) - column2 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 2, ProjectID: 1}) + column2 := unittest.AssertExistsAndLoadBean(t, &Column{ID: 2, ProjectID: 1}) issues, err = column2.GetIssues(db.DefaultContext) assert.NoError(t, err) assert.Len(t, issues, 1) @@ -81,7 +81,7 @@ func Test_MoveColumnsOnProject(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) - columns, err := project1.GetBoards(db.DefaultContext) + columns, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) assert.EqualValues(t, 0, columns[0].Sorting) // even if there is no default sorting, the code should also work @@ -95,7 +95,7 @@ func Test_MoveColumnsOnProject(t *testing.T) { }) assert.NoError(t, err) - columnsAfter, err := project1.GetBoards(db.DefaultContext) + columnsAfter, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columnsAfter, 3) assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) @@ -103,23 +103,23 @@ func Test_MoveColumnsOnProject(t *testing.T) { assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) } -func Test_NewBoard(t *testing.T) { +func Test_NewColumn(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) - columns, err := project1.GetBoards(db.DefaultContext) + columns, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) for i := 0; i < maxProjectColumns-3; i++ { - err := NewBoard(db.DefaultContext, &Board{ - Title: fmt.Sprintf("board-%d", i+4), + err := NewColumn(db.DefaultContext, &Column{ + Title: fmt.Sprintf("column-%d", i+4), ProjectID: project1.ID, }) assert.NoError(t, err) } - err = NewBoard(db.DefaultContext, &Board{ - Title: "board-21", + err = NewColumn(db.DefaultContext, &Column{ + Title: "column-21", ProjectID: project1.ID, }) assert.Error(t, err) diff --git a/models/project/issue.go b/models/project/issue.go index 32e72e909d..3361b533b9 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -18,10 +18,10 @@ type ProjectIssue struct { //revive:disable-line:exported IssueID int64 `xorm:"INDEX"` ProjectID int64 `xorm:"INDEX"` - // ProjectBoardID should not be zero since 1.22. If it's zero, the issue will not be displayed on UI and it might result in errors. - ProjectBoardID int64 `xorm:"INDEX"` + // ProjectColumnID should not be zero since 1.22. If it's zero, the issue will not be displayed on UI and it might result in errors. + ProjectColumnID int64 `xorm:"'project_board_id' INDEX"` - // the sorting order on the board + // the sorting order on the column Sorting int64 `xorm:"NOT NULL DEFAULT 0"` } @@ -76,13 +76,13 @@ func (p *Project) NumOpenIssues(ctx context.Context) int { return int(c) } -// MoveIssuesOnProjectBoard moves or keeps issues in a column and sorts them inside that column -func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs map[int64]int64) error { +// MoveIssuesOnProjectColumn moves or keeps issues in a column and sorts them inside that column +func MoveIssuesOnProjectColumn(ctx context.Context, column *Column, sortedIssueIDs map[int64]int64) error { return db.WithTx(ctx, func(ctx context.Context) error { sess := db.GetEngine(ctx) issueIDs := util.ValuesOfMap(sortedIssueIDs) - count, err := sess.Table(new(ProjectIssue)).Where("project_id=?", board.ProjectID).In("issue_id", issueIDs).Count() + count, err := sess.Table(new(ProjectIssue)).Where("project_id=?", column.ProjectID).In("issue_id", issueIDs).Count() if err != nil { return err } @@ -91,7 +91,7 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs } for sorting, issueID := range sortedIssueIDs { - _, err = sess.Exec("UPDATE `project_issue` SET project_board_id=?, sorting=? WHERE issue_id=?", board.ID, sorting, issueID) + _, err = sess.Exec("UPDATE `project_issue` SET project_board_id=?, sorting=? WHERE issue_id=?", column.ID, sorting, issueID) if err != nil { return err } @@ -100,12 +100,12 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs }) } -func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) error { - if b.ProjectID != newColumn.ProjectID { +func (c *Column) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Column) error { + if c.ProjectID != newColumn.ProjectID { return fmt.Errorf("columns have to be in the same project") } - if b.ID == newColumn.ID { + if c.ID == newColumn.ID { return nil } @@ -121,7 +121,7 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return err } - issues, err := b.GetIssues(ctx) + issues, err := c.GetIssues(ctx) if err != nil { return err } @@ -132,7 +132,7 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) nextSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) return db.WithTx(ctx, func(ctx context.Context) error { for i, issue := range issues { - issue.ProjectBoardID = newColumn.ID + issue.ProjectColumnID = newColumn.ID issue.Sorting = nextSorting + int64(i) if _, err := db.GetEngine(ctx).ID(issue.ID).Cols("project_board_id", "sorting").Update(issue); err != nil { return err diff --git a/models/project/project.go b/models/project/project.go index 8be38694c5..fe5d408f64 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -21,13 +21,7 @@ import ( ) type ( - // BoardConfig is used to identify the type of board that is being created - BoardConfig struct { - BoardType BoardType - Translation string - } - - // CardConfig is used to identify the type of board card that is being used + // CardConfig is used to identify the type of column card that is being used CardConfig struct { CardType CardType Translation string @@ -38,7 +32,7 @@ type ( ) const ( - // TypeIndividual is a type of project board that is owned by an individual + // TypeIndividual is a type of project column that is owned by an individual TypeIndividual Type = iota + 1 // TypeRepository is a project that is tied to a repository @@ -68,39 +62,39 @@ func (err ErrProjectNotExist) Unwrap() error { return util.ErrNotExist } -// ErrProjectBoardNotExist represents a "ProjectBoardNotExist" kind of error. -type ErrProjectBoardNotExist struct { - BoardID int64 +// ErrProjectColumnNotExist represents a "ErrProjectColumnNotExist" kind of error. +type ErrProjectColumnNotExist struct { + ColumnID int64 } -// IsErrProjectBoardNotExist checks if an error is a ErrProjectBoardNotExist -func IsErrProjectBoardNotExist(err error) bool { - _, ok := err.(ErrProjectBoardNotExist) +// IsErrProjectColumnNotExist checks if an error is a ErrProjectColumnNotExist +func IsErrProjectColumnNotExist(err error) bool { + _, ok := err.(ErrProjectColumnNotExist) return ok } -func (err ErrProjectBoardNotExist) Error() string { - return fmt.Sprintf("project board does not exist [id: %d]", err.BoardID) +func (err ErrProjectColumnNotExist) Error() string { + return fmt.Sprintf("project column does not exist [id: %d]", err.ColumnID) } -func (err ErrProjectBoardNotExist) Unwrap() error { +func (err ErrProjectColumnNotExist) Unwrap() error { return util.ErrNotExist } -// Project represents a project board +// Project represents a project type Project struct { - ID int64 `xorm:"pk autoincr"` - Title string `xorm:"INDEX NOT NULL"` - Description string `xorm:"TEXT"` - OwnerID int64 `xorm:"INDEX"` - Owner *user_model.User `xorm:"-"` - RepoID int64 `xorm:"INDEX"` - Repo *repo_model.Repository `xorm:"-"` - CreatorID int64 `xorm:"NOT NULL"` - IsClosed bool `xorm:"INDEX"` - BoardType BoardType - CardType CardType - Type Type + ID int64 `xorm:"pk autoincr"` + Title string `xorm:"INDEX NOT NULL"` + Description string `xorm:"TEXT"` + OwnerID int64 `xorm:"INDEX"` + Owner *user_model.User `xorm:"-"` + RepoID int64 `xorm:"INDEX"` + Repo *repo_model.Repository `xorm:"-"` + CreatorID int64 `xorm:"NOT NULL"` + IsClosed bool `xorm:"INDEX"` + TemplateType TemplateType `xorm:"'board_type'"` // TODO: rename the column to template_type + CardType CardType + Type Type RenderedContent template.HTML `xorm:"-"` @@ -172,16 +166,7 @@ func init() { db.RegisterModel(new(Project)) } -// GetBoardConfig retrieves the types of configurations project boards could have -func GetBoardConfig() []BoardConfig { - return []BoardConfig{ - {BoardTypeNone, "repo.projects.type.none"}, - {BoardTypeBasicKanban, "repo.projects.type.basic_kanban"}, - {BoardTypeBugTriage, "repo.projects.type.bug_triage"}, - } -} - -// GetCardConfig retrieves the types of configurations project board cards could have +// GetCardConfig retrieves the types of configurations project column cards could have func GetCardConfig() []CardConfig { return []CardConfig{ {CardTypeTextOnly, "repo.projects.card_type.text_only"}, @@ -251,8 +236,8 @@ func GetSearchOrderByBySortType(sortType string) db.SearchOrderBy { // NewProject creates a new Project func NewProject(ctx context.Context, p *Project) error { - if !IsBoardTypeValid(p.BoardType) { - p.BoardType = BoardTypeNone + if !IsTemplateTypeValid(p.TemplateType) { + p.TemplateType = TemplateTypeNone } if !IsCardTypeValid(p.CardType) { @@ -263,27 +248,19 @@ func NewProject(ctx context.Context, p *Project) error { return util.NewInvalidArgumentErrorf("project type is not valid") } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := db.Insert(ctx, p); err != nil { - return err - } - - if p.RepoID > 0 { - if _, err := db.Exec(ctx, "UPDATE `repository` SET num_projects = num_projects + 1 WHERE id = ?", p.RepoID); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if err := db.Insert(ctx, p); err != nil { return err } - } - if err := createBoardsForProjectsType(ctx, p); err != nil { - return err - } + if p.RepoID > 0 { + if _, err := db.Exec(ctx, "UPDATE `repository` SET num_projects = num_projects + 1 WHERE id = ?", p.RepoID); err != nil { + return err + } + } - return committer.Commit() + return createDefaultColumnsForProject(ctx, p) + }) } // GetProjectByID returns the projects in a repository @@ -417,7 +394,7 @@ func DeleteProjectByID(ctx context.Context, id int64) error { return err } - if err := deleteBoardByProjectID(ctx, id); err != nil { + if err := deleteColumnByProjectID(ctx, id); err != nil { return err } diff --git a/models/project/project_test.go b/models/project/project_test.go index 8fbbdedecf..dd421b4659 100644 --- a/models/project/project_test.go +++ b/models/project/project_test.go @@ -51,13 +51,13 @@ func TestProject(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) project := &Project{ - Type: TypeRepository, - BoardType: BoardTypeBasicKanban, - CardType: CardTypeTextOnly, - Title: "New Project", - RepoID: 1, - CreatedUnix: timeutil.TimeStampNow(), - CreatorID: 2, + Type: TypeRepository, + TemplateType: TemplateTypeBasicKanban, + CardType: CardTypeTextOnly, + Title: "New Project", + RepoID: 1, + CreatedUnix: timeutil.TimeStampNow(), + CreatorID: 2, } assert.NoError(t, NewProject(db.DefaultContext, project)) diff --git a/models/project/template.go b/models/project/template.go new file mode 100644 index 0000000000..06d5d2af14 --- /dev/null +++ b/models/project/template.go @@ -0,0 +1,45 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +type ( + // TemplateType is used to represent a project template type + TemplateType uint8 + + // TemplateConfig is used to identify the template type of project that is being created + TemplateConfig struct { + TemplateType TemplateType + Translation string + } +) + +const ( + // TemplateTypeNone is a project template type that has no predefined columns + TemplateTypeNone TemplateType = iota + + // TemplateTypeBasicKanban is a project template type that has basic predefined columns + TemplateTypeBasicKanban + + // TemplateTypeBugTriage is a project template type that has predefined columns suited to hunting down bugs + TemplateTypeBugTriage +) + +// GetTemplateConfigs retrieves the template configs of configurations project columns could have +func GetTemplateConfigs() []TemplateConfig { + return []TemplateConfig{ + {TemplateTypeNone, "repo.projects.type.none"}, + {TemplateTypeBasicKanban, "repo.projects.type.basic_kanban"}, + {TemplateTypeBugTriage, "repo.projects.type.bug_triage"}, + } +} + +// IsTemplateTypeValid checks if the project template type is valid +func IsTemplateTypeValid(p TemplateType) bool { + switch p { + case TemplateTypeNone, TemplateTypeBasicKanban, TemplateTypeBugTriage: + return true + default: + return false + } +} diff --git a/models/unit/unit.go b/models/unit/unit.go index 8b4d0caa4c..3beee6a572 100644 --- a/models/unit/unit.go +++ b/models/unit/unit.go @@ -28,7 +28,7 @@ const ( TypeWiki // 5 Wiki TypeExternalWiki // 6 ExternalWiki TypeExternalTracker // 7 ExternalTracker - TypeProjects // 8 Kanban board + TypeProjects // 8 Projects TypePackages // 9 Packages TypeActions // 10 Actions ) diff --git a/modules/indexer/issues/bleve/bleve.go b/modules/indexer/issues/bleve/bleve.go index e3193dbc02..b20fcc6f80 100644 --- a/modules/indexer/issues/bleve/bleve.go +++ b/modules/indexer/issues/bleve/bleve.go @@ -230,8 +230,8 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( if options.ProjectID.Has() { queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectID.Value(), "project_id")) } - if options.ProjectBoardID.Has() { - queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectBoardID.Value(), "project_board_id")) + if options.ProjectColumnID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectColumnID.Value(), "project_board_id")) } if options.PosterID.Has() { diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index eeaf1696ad..875a4ca279 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -61,7 +61,7 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m ReviewedID: convertID(options.ReviewedID), SubscriberID: convertID(options.SubscriberID), ProjectID: convertID(options.ProjectID), - ProjectBoardID: convertID(options.ProjectBoardID), + ProjectColumnID: convertID(options.ProjectColumnID), IsClosed: options.IsClosed, IsPull: options.IsPull, IncludedLabelNames: nil, diff --git a/modules/indexer/issues/dboptions.go b/modules/indexer/issues/dboptions.go index 8f94088742..d9cf9b5e3b 100644 --- a/modules/indexer/issues/dboptions.go +++ b/modules/indexer/issues/dboptions.go @@ -50,7 +50,7 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp } searchOpt.ProjectID = convertID(opts.ProjectID) - searchOpt.ProjectBoardID = convertID(opts.ProjectBoardID) + searchOpt.ProjectColumnID = convertID(opts.ProjectColumnID) searchOpt.PosterID = convertID(opts.PosterID) searchOpt.AssigneeID = convertID(opts.AssigneeID) searchOpt.MentionID = convertID(opts.MentionedID) diff --git a/modules/indexer/issues/elasticsearch/elasticsearch.go b/modules/indexer/issues/elasticsearch/elasticsearch.go index c7cb59f2cf..6f70515009 100644 --- a/modules/indexer/issues/elasticsearch/elasticsearch.go +++ b/modules/indexer/issues/elasticsearch/elasticsearch.go @@ -197,8 +197,8 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( if options.ProjectID.Has() { query.Must(elastic.NewTermQuery("project_id", options.ProjectID.Value())) } - if options.ProjectBoardID.Has() { - query.Must(elastic.NewTermQuery("project_board_id", options.ProjectBoardID.Value())) + if options.ProjectColumnID.Has() { + query.Must(elastic.NewTermQuery("project_board_id", options.ProjectColumnID.Value())) } if options.PosterID.Has() { diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index 0d0cfc8516..e426229f78 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -369,13 +369,13 @@ func searchIssueInProject(t *testing.T) { }, { SearchOptions{ - ProjectBoardID: optional.Some(int64(1)), + ProjectColumnID: optional.Some(int64(1)), }, []int64{1}, }, { SearchOptions{ - ProjectBoardID: optional.Some(int64(0)), // issue with in default board + ProjectColumnID: optional.Some(int64(0)), // issue with in default column }, []int64{2}, }, diff --git a/modules/indexer/issues/internal/model.go b/modules/indexer/issues/internal/model.go index e9c4eca559..2dfee8b72e 100644 --- a/modules/indexer/issues/internal/model.go +++ b/modules/indexer/issues/internal/model.go @@ -27,7 +27,7 @@ type IndexerData struct { NoLabel bool `json:"no_label"` // True if LabelIDs is empty MilestoneID int64 `json:"milestone_id"` ProjectID int64 `json:"project_id"` - ProjectBoardID int64 `json:"project_board_id"` + ProjectColumnID int64 `json:"project_board_id"` // the key should be kept as project_board_id to keep compatible PosterID int64 `json:"poster_id"` AssigneeID int64 `json:"assignee_id"` MentionIDs []int64 `json:"mention_ids"` @@ -89,8 +89,8 @@ type SearchOptions struct { MilestoneIDs []int64 // milestones the issues have - ProjectID optional.Option[int64] // project the issues belong to - ProjectBoardID optional.Option[int64] // project board the issues belong to + ProjectID optional.Option[int64] // project the issues belong to + ProjectColumnID optional.Option[int64] // project column the issues belong to PosterID optional.Option[int64] // poster of the issues diff --git a/modules/indexer/issues/internal/tests/tests.go b/modules/indexer/issues/internal/tests/tests.go index 66e396e02c..5f2488a06e 100644 --- a/modules/indexer/issues/internal/tests/tests.go +++ b/modules/indexer/issues/internal/tests/tests.go @@ -352,38 +352,38 @@ var cases = []*testIndexerCase{ }, }, { - Name: "ProjectBoardID", + Name: "ProjectColumnID", SearchOptions: &internal.SearchOptions{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectBoardID: optional.Some(int64(1)), + ProjectColumnID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) for _, v := range result.Hits { - assert.Equal(t, int64(1), data[v.ID].ProjectBoardID) + assert.Equal(t, int64(1), data[v.ID].ProjectColumnID) } assert.Equal(t, countIndexerData(data, func(v *internal.IndexerData) bool { - return v.ProjectBoardID == 1 + return v.ProjectColumnID == 1 }), result.Total) }, }, { - Name: "no ProjectBoardID", + Name: "no ProjectColumnID", SearchOptions: &internal.SearchOptions{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectBoardID: optional.Some(int64(0)), + ProjectColumnID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) for _, v := range result.Hits { - assert.Equal(t, int64(0), data[v.ID].ProjectBoardID) + assert.Equal(t, int64(0), data[v.ID].ProjectColumnID) } assert.Equal(t, countIndexerData(data, func(v *internal.IndexerData) bool { - return v.ProjectBoardID == 0 + return v.ProjectColumnID == 0 }), result.Total) }, }, @@ -720,7 +720,7 @@ func generateDefaultIndexerData() []*internal.IndexerData { NoLabel: len(labelIDs) == 0, MilestoneID: issueIndex % 4, ProjectID: issueIndex % 5, - ProjectBoardID: issueIndex % 6, + ProjectColumnID: issueIndex % 6, PosterID: id%10 + 1, // PosterID should not be 0 AssigneeID: issueIndex % 10, MentionIDs: mentionIDs, diff --git a/modules/indexer/issues/meilisearch/meilisearch.go b/modules/indexer/issues/meilisearch/meilisearch.go index 8a7cec6cba..9332319339 100644 --- a/modules/indexer/issues/meilisearch/meilisearch.go +++ b/modules/indexer/issues/meilisearch/meilisearch.go @@ -174,8 +174,8 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( if options.ProjectID.Has() { query.And(inner_meilisearch.NewFilterEq("project_id", options.ProjectID.Value())) } - if options.ProjectBoardID.Has() { - query.And(inner_meilisearch.NewFilterEq("project_board_id", options.ProjectBoardID.Value())) + if options.ProjectColumnID.Has() { + query.And(inner_meilisearch.NewFilterEq("project_board_id", options.ProjectColumnID.Value())) } if options.PosterID.Has() { diff --git a/modules/indexer/issues/util.go b/modules/indexer/issues/util.go index 9861c808dc..e752ae6f24 100644 --- a/modules/indexer/issues/util.go +++ b/modules/indexer/issues/util.go @@ -105,7 +105,7 @@ func getIssueIndexerData(ctx context.Context, issueID int64) (*internal.IndexerD NoLabel: len(labels) == 0, MilestoneID: issue.MilestoneID, ProjectID: projectID, - ProjectBoardID: issue.ProjectBoardID(ctx), + ProjectColumnID: issue.ProjectColumnID(ctx), PosterID: issue.PosterID, AssigneeID: issue.AssigneeID, MentionIDs: mentionIDs, diff --git a/modules/metrics/collector.go b/modules/metrics/collector.go index 1bf8f58b93..230260ff94 100755 --- a/modules/metrics/collector.go +++ b/modules/metrics/collector.go @@ -36,7 +36,7 @@ type Collector struct { Oauths *prometheus.Desc Organizations *prometheus.Desc Projects *prometheus.Desc - ProjectBoards *prometheus.Desc + ProjectColumns *prometheus.Desc PublicKeys *prometheus.Desc Releases *prometheus.Desc Repositories *prometheus.Desc @@ -146,9 +146,9 @@ func NewCollector() Collector { "Number of projects", nil, nil, ), - ProjectBoards: prometheus.NewDesc( - namespace+"projects_boards", - "Number of project boards", + ProjectColumns: prometheus.NewDesc( + namespace+"projects_boards", // TODO: change the key name will affect the consume's result history + "Number of project columns", nil, nil, ), PublicKeys: prometheus.NewDesc( @@ -219,7 +219,7 @@ func (c Collector) Describe(ch chan<- *prometheus.Desc) { ch <- c.Oauths ch <- c.Organizations ch <- c.Projects - ch <- c.ProjectBoards + ch <- c.ProjectColumns ch <- c.PublicKeys ch <- c.Releases ch <- c.Repositories @@ -336,9 +336,9 @@ func (c Collector) Collect(ch chan<- prometheus.Metric) { float64(stats.Counter.Project), ) ch <- prometheus.MustNewConstMetric( - c.ProjectBoards, + c.ProjectColumns, prometheus.GaugeValue, - float64(stats.Counter.ProjectBoard), + float64(stats.Counter.ProjectColumn), ) ch <- prometheus.MustNewConstMetric( c.PublicKeys, diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3f3b7143ad..1232ad8d5f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1237,7 +1237,7 @@ tag = Tag tags = Tags issues = Issues pulls = Pull requests -project_board = Projects +project = Projects packages = Packages actions = Actions release = Release diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index e6f3a19625..705a412347 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -34,7 +34,7 @@ const ( // MustEnableProjects check if projects are enabled in settings func MustEnableProjects(ctx *context.Context) { if unit.TypeProjects.UnitGlobalDisabled() { - ctx.NotFound("EnableKanbanBoard", nil) + ctx.NotFound("EnableProjects", nil) return } } @@ -42,7 +42,7 @@ func MustEnableProjects(ctx *context.Context) { // Projects renders the home page of projects func Projects(ctx *context.Context) { shared_user.PrepareContextForProfileBigAvatar(ctx) - ctx.Data["Title"] = ctx.Tr("repo.project_board") + ctx.Data["Title"] = ctx.Tr("repo.projects") sortType := ctx.FormTrim("sort") @@ -139,7 +139,7 @@ func canWriteProjects(ctx *context.Context) bool { // RenderNewProject render creating a project page func RenderNewProject(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.projects.new") - ctx.Data["BoardTypes"] = project_model.GetBoardConfig() + ctx.Data["TemplateConfigs"] = project_model.GetTemplateConfigs() ctx.Data["CardTypes"] = project_model.GetCardConfig() ctx.Data["CanWriteProjects"] = canWriteProjects(ctx) ctx.Data["PageIsViewProjects"] = true @@ -168,12 +168,12 @@ func NewProjectPost(ctx *context.Context) { } newProject := project_model.Project{ - OwnerID: ctx.ContextUser.ID, - Title: form.Title, - Description: form.Content, - CreatorID: ctx.Doer.ID, - BoardType: form.BoardType, - CardType: form.CardType, + OwnerID: ctx.ContextUser.ID, + Title: form.Title, + Description: form.Content, + CreatorID: ctx.Doer.ID, + TemplateType: form.TemplateType, + CardType: form.CardType, } if ctx.ContextUser.IsOrganization() { @@ -314,7 +314,7 @@ func EditProjectPost(ctx *context.Context) { } } -// ViewProject renders the project board for a project +// ViewProject renders the project with board view for a project func ViewProject(ctx *context.Context) { project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { @@ -326,15 +326,15 @@ func ViewProject(ctx *context.Context) { return } - boards, err := project.GetBoards(ctx) + columns, err := project.GetColumns(ctx) if err != nil { - ctx.ServerError("GetProjectBoards", err) + ctx.ServerError("GetProjectColumns", err) return } - issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards) + issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns) if err != nil { - ctx.ServerError("LoadIssuesOfBoards", err) + ctx.ServerError("LoadIssuesOfColumns", err) return } @@ -377,7 +377,7 @@ func ViewProject(ctx *context.Context) { ctx.Data["CanWriteProjects"] = canWriteProjects(ctx) ctx.Data["Project"] = project ctx.Data["IssuesMap"] = issuesMap - ctx.Data["Columns"] = boards // TODO: rename boards to columns in backend + ctx.Data["Columns"] = columns shared_user.RenderUserHeader(ctx) err = shared_user.LoadHeaderCount(ctx) @@ -389,8 +389,8 @@ func ViewProject(ctx *context.Context) { ctx.HTML(http.StatusOK, tplProjectsView) } -// DeleteProjectBoard allows for the deletion of a project board -func DeleteProjectBoard(ctx *context.Context) { +// DeleteProjectColumn allows for the deletion of a project column +func DeleteProjectColumn(ctx *context.Context) { if ctx.Doer == nil { ctx.JSON(http.StatusForbidden, map[string]string{ "message": "Only signed in users are allowed to perform this action.", @@ -404,36 +404,36 @@ func DeleteProjectBoard(ctx *context.Context) { return } - pb, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + pb, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.ServerError("GetProjectColumn", err) return } if pb.ProjectID != ctx.ParamsInt64(":id") { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Project[%d] as expected", pb.ID, project.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Project[%d] as expected", pb.ID, project.ID), }) return } if project.OwnerID != ctx.ContextUser.ID { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Owner[%d] as expected", pb.ID, ctx.ContextUser.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Owner[%d] as expected", pb.ID, ctx.ContextUser.ID), }) return } - if err := project_model.DeleteBoardByID(ctx, ctx.ParamsInt64(":boardID")); err != nil { - ctx.ServerError("DeleteProjectBoardByID", err) + if err := project_model.DeleteColumnByID(ctx, ctx.ParamsInt64(":columnID")); err != nil { + ctx.ServerError("DeleteProjectColumnByID", err) return } ctx.JSONOK() } -// AddBoardToProjectPost allows a new board to be added to a project. -func AddBoardToProjectPost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.EditProjectBoardForm) +// AddColumnToProjectPost allows a new column to be added to a project. +func AddColumnToProjectPost(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.EditProjectColumnForm) project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { @@ -441,21 +441,21 @@ func AddBoardToProjectPost(ctx *context.Context) { return } - if err := project_model.NewBoard(ctx, &project_model.Board{ + if err := project_model.NewColumn(ctx, &project_model.Column{ ProjectID: project.ID, Title: form.Title, Color: form.Color, CreatorID: ctx.Doer.ID, }); err != nil { - ctx.ServerError("NewProjectBoard", err) + ctx.ServerError("NewProjectColumn", err) return } ctx.JSONOK() } -// CheckProjectBoardChangePermissions check permission -func CheckProjectBoardChangePermissions(ctx *context.Context) (*project_model.Project, *project_model.Board) { +// CheckProjectColumnChangePermissions check permission +func CheckProjectColumnChangePermissions(ctx *context.Context) (*project_model.Project, *project_model.Column) { if ctx.Doer == nil { ctx.JSON(http.StatusForbidden, map[string]string{ "message": "Only signed in users are allowed to perform this action.", @@ -469,62 +469,60 @@ func CheckProjectBoardChangePermissions(ctx *context.Context) (*project_model.Pr return nil, nil } - board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + column, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.ServerError("GetProjectColumn", err) return nil, nil } - if board.ProjectID != ctx.ParamsInt64(":id") { + if column.ProjectID != ctx.ParamsInt64(":id") { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Project[%d] as expected", board.ID, project.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Project[%d] as expected", column.ID, project.ID), }) return nil, nil } if project.OwnerID != ctx.ContextUser.ID { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Repository[%d] as expected", board.ID, project.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Repository[%d] as expected", column.ID, project.ID), }) return nil, nil } - return project, board + return project, column } -// EditProjectBoard allows a project board's to be updated -func EditProjectBoard(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.EditProjectBoardForm) - _, board := CheckProjectBoardChangePermissions(ctx) +// EditProjectColumn allows a project column's to be updated +func EditProjectColumn(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.EditProjectColumnForm) + _, column := CheckProjectColumnChangePermissions(ctx) if ctx.Written() { return } if form.Title != "" { - board.Title = form.Title + column.Title = form.Title } - - board.Color = form.Color - + column.Color = form.Color if form.Sorting != 0 { - board.Sorting = form.Sorting + column.Sorting = form.Sorting } - if err := project_model.UpdateBoard(ctx, board); err != nil { - ctx.ServerError("UpdateProjectBoard", err) + if err := project_model.UpdateColumn(ctx, column); err != nil { + ctx.ServerError("UpdateProjectColumn", err) return } ctx.JSONOK() } -// SetDefaultProjectBoard set default board for uncategorized issues/pulls -func SetDefaultProjectBoard(ctx *context.Context) { - project, board := CheckProjectBoardChangePermissions(ctx) +// SetDefaultProjectColumn set default column for uncategorized issues/pulls +func SetDefaultProjectColumn(ctx *context.Context) { + project, column := CheckProjectColumnChangePermissions(ctx) if ctx.Written() { return } - if err := project_model.SetDefaultBoard(ctx, project.ID, board.ID); err != nil { - ctx.ServerError("SetDefaultBoard", err) + if err := project_model.SetDefaultColumn(ctx, project.ID, column.ID); err != nil { + ctx.ServerError("SetDefaultColumn", err) return } @@ -550,14 +548,14 @@ func MoveIssues(ctx *context.Context) { return } - board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + column, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - ctx.NotFoundOrServerError("GetProjectBoard", project_model.IsErrProjectBoardNotExist, err) + ctx.NotFoundOrServerError("GetProjectColumn", project_model.IsErrProjectColumnNotExist, err) return } - if board.ProjectID != project.ID { - ctx.NotFound("BoardNotInProject", nil) + if column.ProjectID != project.ID { + ctx.NotFound("ColumnNotInProject", nil) return } @@ -602,8 +600,8 @@ func MoveIssues(ctx *context.Context) { } } - if err = project_model.MoveIssuesOnProjectBoard(ctx, board, sortedIssueIDs); err != nil { - ctx.ServerError("MoveIssuesOnProjectBoard", err) + if err = project_model.MoveIssuesOnProjectColumn(ctx, column, sortedIssueIDs); err != nil { + ctx.ServerError("MoveIssuesOnProjectColumn", err) return } diff --git a/routers/web/org/projects_test.go b/routers/web/org/projects_test.go index f4ccfe1c06..ab419cc878 100644 --- a/routers/web/org/projects_test.go +++ b/routers/web/org/projects_test.go @@ -13,16 +13,16 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCheckProjectBoardChangePermissions(t *testing.T) { +func TestCheckProjectColumnChangePermissions(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "user2/-/projects/4/4") contexttest.LoadUser(t, ctx, 2) ctx.ContextUser = ctx.Doer // user2 ctx.SetParams(":id", "4") - ctx.SetParams(":boardID", "4") + ctx.SetParams(":columnID", "4") - project, board := org.CheckProjectBoardChangePermissions(ctx) + project, column := org.CheckProjectColumnChangePermissions(ctx) assert.NotNil(t, project) - assert.NotNil(t, board) + assert.NotNil(t, column) assert.False(t, ctx.Written()) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 350e577ede..358554d2ae 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2822,12 +2822,12 @@ func ListIssues(ctx *context.Context) { Page: ctx.FormInt("page"), PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")), }, - Keyword: keyword, - RepoIDs: []int64{ctx.Repo.Repository.ID}, - IsPull: isPull, - IsClosed: isClosed, - ProjectBoardID: projectID, - SortBy: issue_indexer.SortByCreatedDesc, + Keyword: keyword, + RepoIDs: []int64{ctx.Repo.Repository.ID}, + IsPull: isPull, + IsClosed: isClosed, + ProjectID: projectID, + SortBy: issue_indexer.SortByCreatedDesc, } if since != 0 { searchOpt.UpdatedAfterUnix = optional.Some(since) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 934cf8873b..878b7ee699 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -36,7 +36,7 @@ const ( // MustEnableProjects check if projects are enabled in settings func MustEnableProjects(ctx *context.Context) { if unit.TypeProjects.UnitGlobalDisabled() { - ctx.NotFound("EnableKanbanBoard", nil) + ctx.NotFound("EnableRepoProjects", nil) return } @@ -50,7 +50,7 @@ func MustEnableProjects(ctx *context.Context) { // Projects renders the home page of projects func Projects(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("repo.project_board") + ctx.Data["Title"] = ctx.Tr("repo.projects") sortType := ctx.FormTrim("sort") @@ -131,7 +131,7 @@ func Projects(ctx *context.Context) { // RenderNewProject render creating a project page func RenderNewProject(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.projects.new") - ctx.Data["BoardTypes"] = project_model.GetBoardConfig() + ctx.Data["TemplateConfigs"] = project_model.GetTemplateConfigs() ctx.Data["CardTypes"] = project_model.GetCardConfig() ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects) ctx.Data["CancelLink"] = ctx.Repo.Repository.Link() + "/projects" @@ -149,13 +149,13 @@ func NewProjectPost(ctx *context.Context) { } if err := project_model.NewProject(ctx, &project_model.Project{ - RepoID: ctx.Repo.Repository.ID, - Title: form.Title, - Description: form.Content, - CreatorID: ctx.Doer.ID, - BoardType: form.BoardType, - CardType: form.CardType, - Type: project_model.TypeRepository, + RepoID: ctx.Repo.Repository.ID, + Title: form.Title, + Description: form.Content, + CreatorID: ctx.Doer.ID, + TemplateType: form.TemplateType, + CardType: form.CardType, + Type: project_model.TypeRepository, }); err != nil { ctx.ServerError("NewProject", err) return @@ -288,7 +288,7 @@ func EditProjectPost(ctx *context.Context) { } } -// ViewProject renders the project board for a project +// ViewProject renders the project with board view func ViewProject(ctx *context.Context) { project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { @@ -304,15 +304,15 @@ func ViewProject(ctx *context.Context) { return } - boards, err := project.GetBoards(ctx) + columns, err := project.GetColumns(ctx) if err != nil { - ctx.ServerError("GetProjectBoards", err) + ctx.ServerError("GetProjectColumns", err) return } - issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards) + issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns) if err != nil { - ctx.ServerError("LoadIssuesOfBoards", err) + ctx.ServerError("LoadIssuesOfColumns", err) return } @@ -367,7 +367,7 @@ func ViewProject(ctx *context.Context) { ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects) ctx.Data["Project"] = project ctx.Data["IssuesMap"] = issuesMap - ctx.Data["Columns"] = boards // TODO: rename boards to columns in backend + ctx.Data["Columns"] = columns ctx.HTML(http.StatusOK, tplProjectsView) } @@ -405,8 +405,8 @@ func UpdateIssueProject(ctx *context.Context) { ctx.JSONOK() } -// DeleteProjectBoard allows for the deletion of a project board -func DeleteProjectBoard(ctx *context.Context) { +// DeleteProjectColumn allows for the deletion of a project column +func DeleteProjectColumn(ctx *context.Context) { if ctx.Doer == nil { ctx.JSON(http.StatusForbidden, map[string]string{ "message": "Only signed in users are allowed to perform this action.", @@ -431,36 +431,36 @@ func DeleteProjectBoard(ctx *context.Context) { return } - pb, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + pb, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.ServerError("GetProjectColumn", err) return } if pb.ProjectID != ctx.ParamsInt64(":id") { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Project[%d] as expected", pb.ID, project.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Project[%d] as expected", pb.ID, project.ID), }) return } if project.RepoID != ctx.Repo.Repository.ID { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Repository[%d] as expected", pb.ID, ctx.Repo.Repository.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Repository[%d] as expected", pb.ID, ctx.Repo.Repository.ID), }) return } - if err := project_model.DeleteBoardByID(ctx, ctx.ParamsInt64(":boardID")); err != nil { - ctx.ServerError("DeleteProjectBoardByID", err) + if err := project_model.DeleteColumnByID(ctx, ctx.ParamsInt64(":columnID")); err != nil { + ctx.ServerError("DeleteProjectColumnByID", err) return } ctx.JSONOK() } -// AddBoardToProjectPost allows a new board to be added to a project. -func AddBoardToProjectPost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.EditProjectBoardForm) +// AddColumnToProjectPost allows a new column to be added to a project. +func AddColumnToProjectPost(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.EditProjectColumnForm) if !ctx.Repo.IsOwner() && !ctx.Repo.IsAdmin() && !ctx.Repo.CanAccess(perm.AccessModeWrite, unit.TypeProjects) { ctx.JSON(http.StatusForbidden, map[string]string{ "message": "Only authorized users are allowed to perform this action.", @@ -478,20 +478,20 @@ func AddBoardToProjectPost(ctx *context.Context) { return } - if err := project_model.NewBoard(ctx, &project_model.Board{ + if err := project_model.NewColumn(ctx, &project_model.Column{ ProjectID: project.ID, Title: form.Title, Color: form.Color, CreatorID: ctx.Doer.ID, }); err != nil { - ctx.ServerError("NewProjectBoard", err) + ctx.ServerError("NewProjectColumn", err) return } ctx.JSONOK() } -func checkProjectBoardChangePermissions(ctx *context.Context) (*project_model.Project, *project_model.Board) { +func checkProjectColumnChangePermissions(ctx *context.Context) (*project_model.Project, *project_model.Column) { if ctx.Doer == nil { ctx.JSON(http.StatusForbidden, map[string]string{ "message": "Only signed in users are allowed to perform this action.", @@ -516,62 +516,60 @@ func checkProjectBoardChangePermissions(ctx *context.Context) (*project_model.Pr return nil, nil } - board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + column, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.ServerError("GetProjectColumn", err) return nil, nil } - if board.ProjectID != ctx.ParamsInt64(":id") { + if column.ProjectID != ctx.ParamsInt64(":id") { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Project[%d] as expected", board.ID, project.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Project[%d] as expected", column.ID, project.ID), }) return nil, nil } if project.RepoID != ctx.Repo.Repository.ID { ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("ProjectBoard[%d] is not in Repository[%d] as expected", board.ID, ctx.Repo.Repository.ID), + "message": fmt.Sprintf("ProjectColumn[%d] is not in Repository[%d] as expected", column.ID, ctx.Repo.Repository.ID), }) return nil, nil } - return project, board + return project, column } -// EditProjectBoard allows a project board's to be updated -func EditProjectBoard(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.EditProjectBoardForm) - _, board := checkProjectBoardChangePermissions(ctx) +// EditProjectColumn allows a project column's to be updated +func EditProjectColumn(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.EditProjectColumnForm) + _, column := checkProjectColumnChangePermissions(ctx) if ctx.Written() { return } if form.Title != "" { - board.Title = form.Title + column.Title = form.Title } - - board.Color = form.Color - + column.Color = form.Color if form.Sorting != 0 { - board.Sorting = form.Sorting + column.Sorting = form.Sorting } - if err := project_model.UpdateBoard(ctx, board); err != nil { - ctx.ServerError("UpdateProjectBoard", err) + if err := project_model.UpdateColumn(ctx, column); err != nil { + ctx.ServerError("UpdateProjectColumn", err) return } ctx.JSONOK() } -// SetDefaultProjectBoard set default board for uncategorized issues/pulls -func SetDefaultProjectBoard(ctx *context.Context) { - project, board := checkProjectBoardChangePermissions(ctx) +// SetDefaultProjectColumn set default column for uncategorized issues/pulls +func SetDefaultProjectColumn(ctx *context.Context) { + project, column := checkProjectColumnChangePermissions(ctx) if ctx.Written() { return } - if err := project_model.SetDefaultBoard(ctx, project.ID, board.ID); err != nil { - ctx.ServerError("SetDefaultBoard", err) + if err := project_model.SetDefaultColumn(ctx, project.ID, column.ID); err != nil { + ctx.ServerError("SetDefaultColumn", err) return } @@ -608,18 +606,18 @@ func MoveIssues(ctx *context.Context) { return } - board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) + column, err := project_model.GetColumn(ctx, ctx.ParamsInt64(":columnID")) if err != nil { - if project_model.IsErrProjectBoardNotExist(err) { - ctx.NotFound("ProjectBoardNotExist", nil) + if project_model.IsErrProjectColumnNotExist(err) { + ctx.NotFound("ProjectColumnNotExist", nil) } else { - ctx.ServerError("GetProjectBoard", err) + ctx.ServerError("GetProjectColumn", err) } return } - if board.ProjectID != project.ID { - ctx.NotFound("BoardNotInProject", nil) + if column.ProjectID != project.ID { + ctx.NotFound("ColumnNotInProject", nil) return } @@ -663,8 +661,8 @@ func MoveIssues(ctx *context.Context) { } } - if err = project_model.MoveIssuesOnProjectBoard(ctx, board, sortedIssueIDs); err != nil { - ctx.ServerError("MoveIssuesOnProjectBoard", err) + if err = project_model.MoveIssuesOnProjectColumn(ctx, column, sortedIssueIDs); err != nil { + ctx.ServerError("MoveIssuesOnProjectColumn", err) return } diff --git a/routers/web/repo/projects_test.go b/routers/web/repo/projects_test.go index 479f8c55a2..d61230a57e 100644 --- a/routers/web/repo/projects_test.go +++ b/routers/web/repo/projects_test.go @@ -12,16 +12,16 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCheckProjectBoardChangePermissions(t *testing.T) { +func TestCheckProjectColumnChangePermissions(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "user2/repo1/projects/1/2") contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, 1) ctx.SetParams(":id", "1") - ctx.SetParams(":boardID", "2") + ctx.SetParams(":columnID", "2") - project, board := checkProjectBoardChangePermissions(ctx) + project, column := checkProjectColumnChangePermissions(ctx) assert.NotNil(t, project) - assert.NotNil(t, board) + assert.NotNil(t, column) assert.False(t, ctx.Written()) } diff --git a/routers/web/web.go b/routers/web/web.go index a65994c246..34bfb8b60a 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -978,7 +978,7 @@ func registerRoutes(m *web.Route) { m.Get("/new", org.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) m.Group("/{id}", func() { - m.Post("", web.Bind(forms.EditProjectBoardForm{}), org.AddBoardToProjectPost) + m.Post("", web.Bind(forms.EditProjectColumnForm{}), org.AddColumnToProjectPost) m.Post("/move", project.MoveColumns) m.Post("/delete", org.DeleteProject) @@ -986,10 +986,10 @@ func registerRoutes(m *web.Route) { m.Post("/edit", web.Bind(forms.CreateProjectForm{}), org.EditProjectPost) m.Post("/{action:open|close}", org.ChangeProjectStatus) - m.Group("/{boardID}", func() { - m.Put("", web.Bind(forms.EditProjectBoardForm{}), org.EditProjectBoard) - m.Delete("", org.DeleteProjectBoard) - m.Post("/default", org.SetDefaultProjectBoard) + m.Group("/{columnID}", func() { + m.Put("", web.Bind(forms.EditProjectColumnForm{}), org.EditProjectColumn) + m.Delete("", org.DeleteProjectColumn) + m.Post("/default", org.SetDefaultProjectColumn) m.Post("/move", org.MoveIssues) }) @@ -1352,7 +1352,7 @@ func registerRoutes(m *web.Route) { m.Get("/new", repo.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { - m.Post("", web.Bind(forms.EditProjectBoardForm{}), repo.AddBoardToProjectPost) + m.Post("", web.Bind(forms.EditProjectColumnForm{}), repo.AddColumnToProjectPost) m.Post("/move", project.MoveColumns) m.Post("/delete", repo.DeleteProject) @@ -1360,10 +1360,10 @@ func registerRoutes(m *web.Route) { m.Post("/edit", web.Bind(forms.CreateProjectForm{}), repo.EditProjectPost) m.Post("/{action:open|close}", repo.ChangeProjectStatus) - m.Group("/{boardID}", func() { - m.Put("", web.Bind(forms.EditProjectBoardForm{}), repo.EditProjectBoard) - m.Delete("", repo.DeleteProjectBoard) - m.Post("/default", repo.SetDefaultProjectBoard) + m.Group("/{columnID}", func() { + m.Put("", web.Bind(forms.EditProjectColumnForm{}), repo.EditProjectColumn) + m.Delete("", repo.DeleteProjectColumn) + m.Post("/default", repo.SetDefaultProjectColumn) m.Post("/move", repo.MoveIssues) }) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index e826d179ed..0fd8965df9 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -370,45 +370,21 @@ func (i IssueLockForm) HasValidReason() bool { return false } -// __________ __ __ -// \______ \_______ ____ |__| ____ _____/ |_ ______ -// | ___/\_ __ \/ _ \ | |/ __ \_/ ___\ __\/ ___/ -// | | | | \( <_> ) | \ ___/\ \___| | \___ \ -// |____| |__| \____/\__| |\___ >\___ >__| /____ > -// \______| \/ \/ \/ - // CreateProjectForm form for creating a project type CreateProjectForm struct { - Title string `binding:"Required;MaxSize(100)"` - Content string - BoardType project_model.BoardType - CardType project_model.CardType + Title string `binding:"Required;MaxSize(100)"` + Content string + TemplateType project_model.TemplateType + CardType project_model.CardType } -// UserCreateProjectForm is a from for creating an individual or organization -// form. -type UserCreateProjectForm struct { - Title string `binding:"Required;MaxSize(100)"` - Content string - BoardType project_model.BoardType - CardType project_model.CardType - UID int64 `binding:"Required"` -} - -// EditProjectBoardForm is a form for editing a project board -type EditProjectBoardForm struct { +// EditProjectColumnForm is a form for editing a project column +type EditProjectColumnForm struct { Title string `binding:"Required;MaxSize(100)"` Sorting int8 Color string `binding:"MaxSize(7)"` } -// _____ .__.__ __ -// / \ |__| | ____ _______/ |_ ____ ____ ____ -// / \ / \| | | _/ __ \ / ___/\ __\/ _ \ / \_/ __ \ -// / Y \ | |_\ ___/ \___ \ | | ( <_> ) | \ ___/ -// \____|__ /__|____/\___ >____ > |__| \____/|___| /\___ > -// \/ \/ \/ \/ \/ - // CreateMilestoneForm form for creating milestone type CreateMilestoneForm struct { Title string `binding:"Required;MaxSize(50)"` @@ -422,13 +398,6 @@ func (f *CreateMilestoneForm) Validate(req *http.Request, errs binding.Errors) b return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// .____ ___. .__ -// | | _____ \_ |__ ____ | | -// | | \__ \ | __ \_/ __ \| | -// | |___ / __ \| \_\ \ ___/| |__ -// |_______ (____ /___ /\___ >____/ -// \/ \/ \/ \/ - // CreateLabelForm form for creating label type CreateLabelForm struct { ID int64 @@ -456,13 +425,6 @@ func (f *InitializeLabelsForm) Validate(req *http.Request, errs binding.Errors) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// __________ .__ .__ __________ __ -// \______ \__ __| | | | \______ \ ____ ________ __ ____ _______/ |_ -// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ -// | | | | / |_| |__ | | \ ___< <_| | | /\ ___/ \___ \ | | -// |____| |____/|____/____/ |____|_ /\___ >__ |____/ \___ >____ > |__| -// \/ \/ |__| \/ \/ - // MergePullRequestForm form for merging Pull Request // swagger:model MergePullRequestOption type MergePullRequestForm struct { diff --git a/services/forms/user_form_hidden_comments.go b/services/forms/user_form_hidden_comments.go index c21fddf478..b9677c1800 100644 --- a/services/forms/user_form_hidden_comments.go +++ b/services/forms/user_form_hidden_comments.go @@ -65,7 +65,7 @@ var hiddenCommentTypeGroups = hiddenCommentTypeGroupsType{ }, "project": { /*30*/ issues_model.CommentTypeProject, - /*31*/ issues_model.CommentTypeProjectBoard, + /*31*/ issues_model.CommentTypeProjectColumn, }, "issue_ref": { /*33*/ issues_model.CommentTypeChangeIssueRef, diff --git a/templates/projects/new.tmpl b/templates/projects/new.tmpl index 92ee36c1c4..bd173b54bc 100644 --- a/templates/projects/new.tmpl +++ b/templates/projects/new.tmpl @@ -25,11 +25,11 @@
- +
{{ctx.Locale.Tr "repo.projects.template.desc_helper"}}
- {{range $element := .BoardTypes}} -
{{ctx.Locale.Tr $element.Translation}}
+ {{range $element := .TemplateConfigs}} +
{{ctx.Locale.Tr $element.Translation}}
{{end}}
diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 21017415c1..82e7679cd2 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -131,7 +131,7 @@ {{if and (not .UnitProjectsGlobalDisabled) (.Permission.CanRead $.UnitTypeProjects)}} - {{svg "octicon-project"}} {{ctx.Locale.Tr "repo.project_board"}} + {{svg "octicon-project"}} {{ctx.Locale.Tr "repo.project"}} {{if .Repository.NumOpenProjects}} {{CountFmt .Repository.NumOpenProjects}} {{end}} diff --git a/templates/repo/issue/filter_actions.tmpl b/templates/repo/issue/filter_actions.tmpl index d559f335b7..a341448bcc 100644 --- a/templates/repo/issue/filter_actions.tmpl +++ b/templates/repo/issue/filter_actions.tmpl @@ -71,7 +71,7 @@
- {{ctx.Locale.Tr "repo.project_board"}} + {{ctx.Locale.Tr "repo.projects"}} {{svg "octicon-triangle-down" 14 "dropdown icon"}}
diff --git a/templates/repo/settings/units/overview.tmpl b/templates/repo/settings/units/overview.tmpl index 816b45ce1d..a851c66d5a 100644 --- a/templates/repo/settings/units/overview.tmpl +++ b/templates/repo/settings/units/overview.tmpl @@ -15,7 +15,7 @@ {{$isProjectsEnabled := .Repository.UnitEnabled $.Context $.UnitTypeProjects}} {{$isProjectsGlobalDisabled := .UnitTypeProjects.UnitGlobalDisabled}}
- +
diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 5ddea4314a..09f47baae8 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -35,23 +35,23 @@ func TestMoveRepoProjectColumns(t *testing.T) { repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) project1 := project_model.Project{ - Title: "new created project", - RepoID: repo2.ID, - Type: project_model.TypeRepository, - BoardType: project_model.BoardTypeNone, + Title: "new created project", + RepoID: repo2.ID, + Type: project_model.TypeRepository, + TemplateType: project_model.TemplateTypeNone, } err := project_model.NewProject(db.DefaultContext, &project1) assert.NoError(t, err) for i := 0; i < 3; i++ { - err = project_model.NewBoard(db.DefaultContext, &project_model.Board{ + err = project_model.NewColumn(db.DefaultContext, &project_model.Column{ Title: fmt.Sprintf("column %d", i+1), ProjectID: project1.ID, }) assert.NoError(t, err) } - columns, err := project1.GetBoards(db.DefaultContext) + columns, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) assert.EqualValues(t, 0, columns[0].Sorting) @@ -72,7 +72,7 @@ func TestMoveRepoProjectColumns(t *testing.T) { }) sess.MakeRequest(t, req, http.StatusOK) - columnsAfter, err := project1.GetBoards(db.DefaultContext) + columnsAfter, err := project1.GetColumns(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) diff --git a/web_src/css/features/projects.css b/web_src/css/features/projects.css index 7ef9d72e17..d363b97e22 100644 --- a/web_src/css/features/projects.css +++ b/web_src/css/features/projects.css @@ -7,7 +7,7 @@ } .project-column { - background-color: var(--color-project-board-bg) !important; + background-color: var(--color-project-column-bg) !important; border: 1px solid var(--color-secondary) !important; margin: 0 0.5rem !important; padding: 0.5rem !important; diff --git a/web_src/css/themes/theme-gitea-dark.css b/web_src/css/themes/theme-gitea-dark.css index f226f2264c..1763f236cf 100644 --- a/web_src/css/themes/theme-gitea-dark.css +++ b/web_src/css/themes/theme-gitea-dark.css @@ -218,7 +218,7 @@ --color-expand-button: #2b353e; --color-placeholder-text: var(--color-text-light-3); --color-editor-line-highlight: var(--color-primary-light-5); - --color-project-board-bg: var(--color-secondary-light-2); + --color-project-column-bg: var(--color-secondary-light-2); --color-caret: var(--color-text); /* should ideally be --color-text-dark, see #15651 */ --color-reaction-bg: #e8e8ff12; --color-reaction-hover-bg: var(--color-primary-light-4); diff --git a/web_src/css/themes/theme-gitea-light.css b/web_src/css/themes/theme-gitea-light.css index f266e3583d..fd348dfdc2 100644 --- a/web_src/css/themes/theme-gitea-light.css +++ b/web_src/css/themes/theme-gitea-light.css @@ -218,7 +218,7 @@ --color-expand-button: #cfe8fa; --color-placeholder-text: var(--color-text-light-3); --color-editor-line-highlight: var(--color-primary-light-6); - --color-project-board-bg: var(--color-secondary-light-4); + --color-project-column-bg: var(--color-secondary-light-4); --color-caret: var(--color-text-dark); --color-reaction-bg: #0000170a; --color-reaction-hover-bg: var(--color-primary-light-5); From 73706ae26d138684ef9da9e1164846a040fd4a7d Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Mon, 27 May 2024 07:56:04 -0500 Subject: [PATCH 05/16] feat: add support for a credentials chain for minio access (#31051) We wanted to be able to use the IAM role provided by the EC2 instance metadata in order to access S3 via the Minio configuration. To do this, a new credentials chain is added that will check the following locations for credentials when an access key is not provided. In priority order, they are: 1. MINIO_ prefixed environment variables 2. AWS_ prefixed environment variables 3. a minio credentials file 4. an aws credentials file 5. EC2 instance metadata (cherry picked from commit c0880e7695346997c6a93f05cd01634cb3ad03ee) Conflicts: docs/content/administration/config-cheat-sheet.en-us.md does not exist in Forgejo --- custom/conf/app.example.ini | 10 ++- modules/storage/minio.go | 31 ++++++- modules/storage/minio_test.go | 104 +++++++++++++++++++++++ modules/storage/testdata/aws_credentials | 3 + modules/storage/testdata/minio.json | 12 +++ 5 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 modules/storage/testdata/aws_credentials create mode 100644 modules/storage/testdata/minio.json diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 913c0e7496..d825e47a76 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1924,7 +1924,10 @@ LEVEL = Info ;; Minio endpoint to connect only available when STORAGE_TYPE is `minio` ;MINIO_ENDPOINT = localhost:9000 ;; -;; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio` +;; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for credentials in known +;; environment variables (MINIO_ACCESS_KEY_ID, AWS_ACCESS_KEY_ID), credentials files +;; (~/.mc/config.json, ~/.aws/credentials), and EC2 instance metadata. ;MINIO_ACCESS_KEY_ID = ;; ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` @@ -2633,7 +2636,10 @@ LEVEL = Info ;; Minio endpoint to connect only available when STORAGE_TYPE is `minio` ;MINIO_ENDPOINT = localhost:9000 ;; -;; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio` +;; Minio accessKeyID to connect only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for credentials in known +;; environment variables (MINIO_ACCESS_KEY_ID, AWS_ACCESS_KEY_ID), credentials files +;; (~/.mc/config.json, ~/.aws/credentials), and EC2 instance metadata. ;MINIO_ACCESS_KEY_ID = ;; ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 0b65577cb5..d0c2dec65b 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -97,7 +97,7 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath) minioClient, err := minio.New(config.Endpoint, &minio.Options{ - Creds: credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""), + Creds: buildMinioCredentials(config, credentials.DefaultIAMRoleEndpoint), Secure: config.UseSSL, Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipVerify}}, Region: config.Location, @@ -164,6 +164,35 @@ func (m *MinioStorage) buildMinioDirPrefix(p string) string { return p } +func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string) *credentials.Credentials { + // If static credentials are provided, use those + if config.AccessKeyID != "" { + return credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, "") + } + + // Otherwise, fallback to a credentials chain for S3 access + chain := []credentials.Provider{ + // configure based upon MINIO_ prefixed environment variables + &credentials.EnvMinio{}, + // configure based upon AWS_ prefixed environment variables + &credentials.EnvAWS{}, + // read credentials from MINIO_SHARED_CREDENTIALS_FILE + // environment variable, or default json config files + &credentials.FileMinioClient{}, + // read credentials from AWS_SHARED_CREDENTIALS_FILE + // environment variable, or default credentials file + &credentials.FileAWSCredentials{}, + // read IAM role from EC2 metadata endpoint if available + &credentials.IAM{ + Endpoint: iamEndpoint, + Client: &http.Client{ + Transport: http.DefaultTransport, + }, + }, + } + return credentials.NewChainCredentials(chain) +} + // Open opens a file func (m *MinioStorage) Open(path string) (Object, error) { opts := minio.GetObjectOptions{} diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 2e1a3028c7..3fe01825e9 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -6,6 +6,7 @@ package storage import ( "context" "net/http" + "net/http/httptest" "os" "testing" @@ -109,3 +110,106 @@ func TestS3StorageBadRequest(t *testing.T) { _, err := NewStorage(setting.MinioStorageType, cfg) assert.ErrorContains(t, err, message) } + +func TestMinioCredentials(t *testing.T) { + const ( + ExpectedAccessKey = "ExampleAccessKeyID" + ExpectedSecretAccessKey = "ExampleSecretAccessKeyID" + // Use a FakeEndpoint for IAM credentials to avoid logging any + // potential real IAM credentials when running in EC2. + FakeEndpoint = "http://localhost" + ) + + t.Run("Static Credentials", func(t *testing.T) { + cfg := setting.MinioStorageConfig{ + AccessKeyID: ExpectedAccessKey, + SecretAccessKey: ExpectedSecretAccessKey, + } + creds := buildMinioCredentials(cfg, FakeEndpoint) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey, v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey, v.SecretAccessKey) + }) + + t.Run("Chain", func(t *testing.T) { + cfg := setting.MinioStorageConfig{} + + t.Run("EnvMinio", func(t *testing.T) { + t.Setenv("MINIO_ACCESS_KEY", ExpectedAccessKey+"Minio") + t.Setenv("MINIO_SECRET_KEY", ExpectedSecretAccessKey+"Minio") + + creds := buildMinioCredentials(cfg, FakeEndpoint) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey+"Minio", v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey+"Minio", v.SecretAccessKey) + }) + + t.Run("EnvAWS", func(t *testing.T) { + t.Setenv("AWS_ACCESS_KEY", ExpectedAccessKey+"AWS") + t.Setenv("AWS_SECRET_KEY", ExpectedSecretAccessKey+"AWS") + + creds := buildMinioCredentials(cfg, FakeEndpoint) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey+"AWS", v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey+"AWS", v.SecretAccessKey) + }) + + t.Run("FileMinio", func(t *testing.T) { + t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json") + // prevent loading any actual credentials files from the user + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/fake") + + creds := buildMinioCredentials(cfg, FakeEndpoint) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey+"MinioFile", v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey+"MinioFile", v.SecretAccessKey) + }) + + t.Run("FileAWS", func(t *testing.T) { + // prevent loading any actual credentials files from the user + t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/fake.json") + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/aws_credentials") + + creds := buildMinioCredentials(cfg, FakeEndpoint) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey+"AWSFile", v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey+"AWSFile", v.SecretAccessKey) + }) + + t.Run("IAM", func(t *testing.T) { + // prevent loading any actual credentials files from the user + t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/fake.json") + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/fake") + + // Spawn a server to emulate the EC2 Instance Metadata + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The client will actually make 3 requests here, + // first will be to get the IMDSv2 token, second to + // get the role, and third for the actual + // credentials. However, we can return credentials + // every request since we're not emulating a full + // IMDSv2 flow. + w.Write([]byte(`{"Code":"Success","AccessKeyId":"ExampleAccessKeyIDIAM","SecretAccessKey":"ExampleSecretAccessKeyIDIAM"}`)) + })) + defer server.Close() + + // Use the provided EC2 Instance Metadata server + creds := buildMinioCredentials(cfg, server.URL) + v, err := creds.Get() + + assert.NoError(t, err) + assert.Equal(t, ExpectedAccessKey+"IAM", v.AccessKeyID) + assert.Equal(t, ExpectedSecretAccessKey+"IAM", v.SecretAccessKey) + }) + }) +} diff --git a/modules/storage/testdata/aws_credentials b/modules/storage/testdata/aws_credentials new file mode 100644 index 0000000000..62a5488b51 --- /dev/null +++ b/modules/storage/testdata/aws_credentials @@ -0,0 +1,3 @@ +[default] +aws_access_key_id=ExampleAccessKeyIDAWSFile +aws_secret_access_key=ExampleSecretAccessKeyIDAWSFile diff --git a/modules/storage/testdata/minio.json b/modules/storage/testdata/minio.json new file mode 100644 index 0000000000..3876257626 --- /dev/null +++ b/modules/storage/testdata/minio.json @@ -0,0 +1,12 @@ +{ + "version": "10", + "aliases": { + "s3": { + "url": "https://s3.amazonaws.com", + "accessKey": "ExampleAccessKeyIDMinioFile", + "secretKey": "ExampleSecretAccessKeyIDMinioFile", + "api": "S3v4", + "path": "dns" + } + } +} From ca0921a95aa9a37d8820538458c15fd0a3b0c97c Mon Sep 17 00:00:00 2001 From: metiftikci Date: Mon, 27 May 2024 18:34:18 +0300 Subject: [PATCH 06/16] Prevent simultaneous editing of comments and issues (#31053) fixes #22907 Tested: - [x] issue content edit - [x] issue content change tasklist - [x] pull request content edit - [x] pull request change tasklist ![issue-content-edit](https://github.com/go-gitea/gitea/assets/29250154/a0828889-fb96-4bc4-8600-da92e3205812) (cherry picked from commit aa92b13164e84c26be91153b6022220ce0a27720) Conflicts: models/issues/comment.go c7a389f2b2667b7bfc264a0be519b5c85e0a3b94 [FEAT] allow setting the update date on issues and comments options/locale/locale_en-US.ini trivial context conflicts routers/api/v1/repo/issue_comment.go routers/api/v1/repo/issue_comment_attachment.go services/issue/comments.go services/issue/content.go user blocking is implemented differently in Forgejo routers/web/repo/issue.go trivial difference from 6a0750177f Allow to save empty comment user blocking is implemented differently in Forgejo templates/repo/issue/view_content/conversation.tmpl templates changed a lot in Forgejo but the change is trivially ported tests/integration/issue_test.go other tests were added in the same region web_src/js/features/repo-issue-edit.js the code is still web_src/js/features/repo-legacy.js trivially ported --- models/issues/comment.go | 13 +++- models/issues/issue.go | 3 + models/issues/issue_update.go | 21 ++++-- models/migrations/migrations.go | 4 ++ models/migrations/v1_23/v299.go | 18 +++++ options/locale/locale_en-US.ini | 4 ++ routers/api/v1/repo/issue.go | 7 +- routers/api/v1/repo/issue_attachment.go | 2 +- routers/api/v1/repo/issue_comment.go | 2 +- .../api/v1/repo/issue_comment_attachment.go | 2 +- routers/api/v1/repo/pull.go | 7 +- routers/web/repo/issue.go | 35 ++++++--- services/issue/comments.go | 4 +- services/issue/content.go | 4 +- templates/repo/diff/comments.tmpl | 2 +- templates/repo/issue/view_content.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 4 +- .../repo/issue/view_content/conversation.tmpl | 2 +- tests/integration/issue_test.go | 71 ++++++++++++++++++- web_src/js/features/repo-legacy.js | 7 ++ web_src/js/markup/tasklist.js | 12 +++- 21 files changed, 190 insertions(+), 36 deletions(-) create mode 100644 models/migrations/v1_23/v299.go diff --git a/models/issues/comment.go b/models/issues/comment.go index b0f7c47a73..d53e5f5949 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -52,6 +52,8 @@ func (err ErrCommentNotExist) Unwrap() error { return util.ErrNotExist } +var ErrCommentAlreadyChanged = util.NewInvalidArgumentErrorf("the comment is already changed") + // CommentType defines whether a comment is just a simple comment, an action (like close) or a reference. type CommentType int @@ -262,6 +264,7 @@ type Comment struct { Line int64 // - previous line / + proposed line TreePath string Content string `xorm:"LONGTEXT"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` RenderedContent template.HTML `xorm:"-"` // Path represents the 4 lines of code cemented by this comment @@ -1119,7 +1122,7 @@ func UpdateCommentInvalidate(ctx context.Context, c *Comment) error { } // UpdateComment updates information of comment. -func UpdateComment(ctx context.Context, c *Comment, doer *user_model.User) error { +func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *user_model.User) error { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -1139,9 +1142,15 @@ func UpdateComment(ctx context.Context, c *Comment, doer *user_model.User) error // see https://codeberg.org/forgejo/forgejo/pulls/764#issuecomment-1023801 c.UpdatedUnix = c.Issue.UpdatedUnix } - if _, err := sess.Update(c); err != nil { + c.ContentVersion = contentVersion + 1 + + affected, err := sess.Where("content_version = ?", contentVersion).Update(c) + if err != nil { return err } + if affected == 0 { + return ErrCommentAlreadyChanged + } if err := c.AddCrossReferences(ctx, doer, true); err != nil { return err } diff --git a/models/issues/issue.go b/models/issues/issue.go index affd581929..349b7e3d05 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -94,6 +94,8 @@ func (err ErrIssueWasClosed) Error() string { return fmt.Sprintf("Issue [%d] %d was already closed", err.ID, err.Index) } +var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already changed") + // Issue represents an issue or pull request of repository. type Issue struct { ID int64 `xorm:"pk autoincr"` @@ -107,6 +109,7 @@ type Issue struct { Title string `xorm:"name"` Content string `xorm:"LONGTEXT"` RenderedContent template.HTML `xorm:"-"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` Labels []*Label `xorm:"-"` MilestoneID int64 `xorm:"INDEX"` Milestone *Milestone `xorm:"-"` diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index c3debac92e..dbfd2fc91b 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -25,17 +25,18 @@ import ( "xorm.io/builder" ) -// UpdateIssueCols updates cols of issue func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error { + _, err := UpdateIssueColsWithCond(ctx, issue, builder.NewCond(), cols...) + return err +} + +func UpdateIssueColsWithCond(ctx context.Context, issue *Issue, cond builder.Cond, cols ...string) (int64, error) { sess := db.GetEngine(ctx).ID(issue.ID) if issue.NoAutoTime { cols = append(cols, []string{"updated_unix"}...) sess.NoAutoTime() } - if _, err := sess.Cols(cols...).Update(issue); err != nil { - return err - } - return nil + return sess.Cols(cols...).Where(cond).Update(issue) } func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) { @@ -250,7 +251,7 @@ func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) } // ChangeIssueContent changes issue content, as the given user. -func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string) (err error) { +func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string, contentVersion int) (err error) { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -269,10 +270,16 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content + issue.ContentVersion = contentVersion + 1 - if err = UpdateIssueCols(ctx, issue, "content"); err != nil { + expectedContentVersion := builder.NewCond().And(builder.Eq{"content_version": contentVersion}) + affected, err := UpdateIssueColsWithCond(ctx, issue, expectedContentVersion, "content", "content_version") + if err != nil { return fmt.Errorf("UpdateIssueCols: %w", err) } + if affected == 0 { + return ErrIssueAlreadyChanged + } historyDate := timeutil.TimeStampNow() if issue.NoAutoTime { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 5d80f9fd10..2e095c05a4 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/models/migrations/v1_20" "code.gitea.io/gitea/models/migrations/v1_21" "code.gitea.io/gitea/models/migrations/v1_22" + "code.gitea.io/gitea/models/migrations/v1_23" "code.gitea.io/gitea/models/migrations/v1_6" "code.gitea.io/gitea/models/migrations/v1_7" "code.gitea.io/gitea/models/migrations/v1_8" @@ -589,6 +590,9 @@ var migrations = []Migration{ NewMigration("Drop wrongly created table o_auth2_application", v1_22.DropWronglyCreatedTable), // Gitea 1.22.0-rc1 ends at 299 + + // v299 -> v300 + NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go new file mode 100644 index 0000000000..f6db960c3b --- /dev/null +++ b/models/migrations/v1_23/v299.go @@ -0,0 +1,18 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +func AddContentVersionToIssueAndComment(x *xorm.Engine) error { + type Issue struct { + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` + } + + type Comment struct { + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` + } + + return x.Sync(new(Comment), new(Issue)) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1232ad8d5f..96dccccf3f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1474,6 +1474,7 @@ issues.new.assignees = Assignees issues.new.clear_assignees = Clear assignees issues.new.no_assignees = No assignees issues.new.no_reviewers = No reviewers +issues.edit.already_changed = Unable to save changes to the issue. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes issues.choose.get_started = Get started issues.choose.open_external_link = Open issues.choose.blank = Default @@ -1791,6 +1792,7 @@ compare.compare_head = compare pulls.desc = Enable pull requests and code reviews. pulls.new = New pull request pulls.view = View pull request +pulls.edit.already_changed = Unable to save changes to the pull request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes pulls.compare_changes = New pull request pulls.allow_edits_from_maintainers = Allow edits from maintainers pulls.allow_edits_from_maintainers_desc = Users with write access to the base branch can also push to this branch @@ -1946,6 +1948,8 @@ pulls.recently_pushed_new_branches = You pushed on branch
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index bb8863a170..9db480bca2 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -60,7 +60,7 @@ {{end}}
{{.Issue.Content}}
-
+
{{if .Issue.Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Issue.Attachments "RenderedContent" .Issue.RenderedContent}} {{end}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index e9e92db07d..5aeb0ff94f 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -67,7 +67,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} @@ -441,7 +441,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index a28956937b..6ba9e73d3d 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -93,7 +93,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 83d353c935..d5bdea2a19 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -282,6 +282,34 @@ func TestIssueDependencies(t *testing.T) { }) } +func TestEditIssue(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description") + + req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusOK) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "content_version": "1", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusOK) +} + func TestIssueCommentClose(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") @@ -399,8 +427,9 @@ func TestIssueCommentUpdate(t *testing.T) { // make the comment empty req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), - "content": "", + "_csrf": GetCSRF(t, session, issueURL), + "content": "", + "content_version": fmt.Sprintf("%d", comment.ContentVersion), }) session.MakeRequest(t, req, http.StatusOK) @@ -408,6 +437,44 @@ func TestIssueCommentUpdate(t *testing.T) { assert.Equal(t, "", comment.Content) } +func TestIssueCommentUpdateSimultaneously(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description") + comment1 := "Test comment 1" + commentID := testIssueAddComment(t, session, issueURL, comment1, "") + + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID}) + assert.Equal(t, comment1, comment.Content) + + modifiedContent := comment.Content + "MODIFIED" + + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + }) + session.MakeRequest(t, req, http.StatusOK) + + modifiedContent = comment.Content + "2" + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + }) + session.MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + "content_version": "1", + }) + session.MakeRequest(t, req, http.StatusOK) + + comment = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID}) + assert.Equal(t, modifiedContent, comment.Content) + assert.Equal(t, 2, comment.ContentVersion) +} + func TestIssueReaction(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index f172000842..a14241b5a6 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -16,6 +16,7 @@ import {initCitationFileCopyContent} from './citation.js'; import {initCompLabelEdit} from './comp/LabelEdit.js'; import {initRepoDiffConversationNav} from './repo-diff.js'; import {createDropzone} from './dropzone.js'; +import {showErrorToast} from '../modules/toast.js'; import {initCommentContent, initMarkupContent} from '../markup/content.js'; import {initCompReactionSelector} from './comp/ReactionSelector.js'; import {initRepoSettingBranches} from './repo-settings.js'; @@ -431,11 +432,17 @@ async function onEditContent(event) { const params = new URLSearchParams({ content: comboMarkdownEditor.value(), context: editContentZone.getAttribute('data-context'), + content_version: editContentZone.getAttribute('data-content-version'), }); for (const fileInput of dropzoneInst?.element.querySelectorAll('.files [name=files]')) params.append('files[]', fileInput.value); const response = await POST(editContentZone.getAttribute('data-update-url'), {data: params}); const data = await response.json(); + if (response.status === 400) { + showErrorToast(data.errorMessage); + return; + } + editContentZone.setAttribute('data-content-version', data.contentVersion); if (!data.content) { renderContent.innerHTML = document.getElementById('no-content').innerHTML; rawContent.textContent = ''; diff --git a/web_src/js/markup/tasklist.js b/web_src/js/markup/tasklist.js index 00076bce58..a40b5e4abd 100644 --- a/web_src/js/markup/tasklist.js +++ b/web_src/js/markup/tasklist.js @@ -1,4 +1,5 @@ import {POST} from '../modules/fetch.js'; +import {showErrorToast} from '../modules/toast.js'; const preventListener = (e) => e.preventDefault(); @@ -54,13 +55,20 @@ export function initMarkupTasklist() { const editContentZone = container.querySelector('.edit-content-zone'); const updateUrl = editContentZone.getAttribute('data-update-url'); const context = editContentZone.getAttribute('data-context'); + const contentVersion = editContentZone.getAttribute('data-content-version'); const requestBody = new FormData(); requestBody.append('ignore_attachments', 'true'); requestBody.append('content', newContent); requestBody.append('context', context); - await POST(updateUrl, {data: requestBody}); - + requestBody.append('content_version', contentVersion); + const response = await POST(updateUrl, {data: requestBody}); + const data = await response.json(); + if (response.status === 400) { + showErrorToast(data.errorMessage); + return; + } + editContentZone.setAttribute('data-content-version', data.contentVersion); rawContent.textContent = newContent; } catch (err) { checkbox.checked = !checkbox.checked; From 344bf48c9b407a2e8bd5eb782a847adfa1f9c569 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 May 2024 17:31:59 +0800 Subject: [PATCH 07/16] Add missed return after `ctx.ServerError` (#31130) (cherry picked from commit b6f15c7948ac3d09977350de83ec91d5789ea083) --- routers/api/v1/repo/mirror.go | 1 + routers/web/admin/repos.go | 1 + routers/web/auth/auth.go | 1 + routers/web/org/projects.go | 1 + routers/web/repo/editor.go | 1 + 5 files changed, 5 insertions(+) diff --git a/routers/api/v1/repo/mirror.go b/routers/api/v1/repo/mirror.go index 2a896de4fe..eddd449206 100644 --- a/routers/api/v1/repo/mirror.go +++ b/routers/api/v1/repo/mirror.go @@ -383,6 +383,7 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro if err = mirror_service.AddPushMirrorRemote(ctx, pushMirror, address); err != nil { if err := repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: pushMirror.ID, RepoID: pushMirror.RepoID}); err != nil { ctx.ServerError("DeletePushMirrors", err) + return } ctx.ServerError("AddPushMirrorRemote", err) return diff --git a/routers/web/admin/repos.go b/routers/web/admin/repos.go index ddf4440167..d0339fdd93 100644 --- a/routers/web/admin/repos.go +++ b/routers/web/admin/repos.go @@ -94,6 +94,7 @@ func UnadoptedRepos(ctx *context.Context) { repoNames, count, err := repo_service.ListUnadoptedRepositories(ctx, q, &opts) if err != nil { ctx.ServerError("ListUnadoptedRepositories", err) + return } ctx.Data["Dirs"] = repoNames pager := context.NewPagination(count, opts.PageSize, opts.Page, 5) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 50fd760d80..a738de380a 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -840,6 +840,7 @@ func ActivateEmail(ctx *context.Context) { if email := user_model.VerifyActiveEmailCode(ctx, code, emailStr); email != nil { if err := user_model.ActivateEmail(ctx, email); err != nil { ctx.ServerError("ActivateEmail", err) + return } log.Trace("Email activated: %s", email.Email) diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 705a412347..64d233fc45 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -569,6 +569,7 @@ func MoveIssues(ctx *context.Context) { form := &movedIssuesForm{} if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { ctx.ServerError("DecodeMovedIssuesForm", err) + return } issueIDs := make([]int64, 0, len(form.Issues)) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index e8439cb40e..00c3d880a9 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -604,6 +604,7 @@ func DeleteFilePost(ctx *context.Context) { } else { ctx.ServerError("DeleteRepoFile", err) } + return } ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath)) From ee2247d77c0b13b0b45df704d7589b541db03899 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 May 2024 18:03:54 +0800 Subject: [PATCH 08/16] Add topics for repository API (#31127) Fix ##31100 (cherry picked from commit de4616690f742aebc3e019fde5c73c432d543292) --- modules/structs/repo.go | 1 + services/convert/repository.go | 1 + templates/swagger/v1_json.tmpl | 7 +++++++ 3 files changed, 9 insertions(+) diff --git a/modules/structs/repo.go b/modules/structs/repo.go index a2e66266af..2aa4136597 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -114,6 +114,7 @@ type Repository struct { // swagger:strfmt date-time MirrorUpdated time.Time `json:"mirror_updated,omitempty"` RepoTransfer *RepoTransfer `json:"repo_transfer"` + Topics []string `json:"topics"` } // GetName implements the gitrepo.Repository interface diff --git a/services/convert/repository.go b/services/convert/repository.go index 35becd96d0..2fb6f6d7c0 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -237,6 +237,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR MirrorInterval: mirrorInterval, MirrorUpdated: mirrorUpdated, RepoTransfer: transfer, + Topics: repo.Topics, ObjectFormatName: repo.ObjectFormatName, } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 67c3f498bf..6a488559a0 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24234,6 +24234,13 @@ "type": "boolean", "x-go-name": "Template" }, + "topics": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "Topics" + }, "updated_at": { "type": "string", "format": "date-time", From 18eef3b23a45d50da94d39a9559388b4b53545a8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 31 May 2024 09:58:41 +0800 Subject: [PATCH 09/16] Fix branch order (#31174) Fix #31172 The original order or the default order should not be ignored even if we have an is_deleted order. (cherry picked from commit 1137a0357eb1e35a046e86a7277594154d0f6c85) --- models/git/branch_list.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/models/git/branch_list.go b/models/git/branch_list.go index 493611f217..81a43eaea3 100644 --- a/models/git/branch_list.go +++ b/models/git/branch_list.go @@ -88,17 +88,13 @@ func (opts FindBranchOptions) ToConds() builder.Cond { func (opts FindBranchOptions) ToOrders() string { orderBy := opts.OrderBy - if opts.IsDeletedBranch.ValueOrDefault(true) { // if deleted branch included, put them at the end - if orderBy != "" { - orderBy += ", " - } - orderBy += "is_deleted ASC" - } if orderBy == "" { // the commit_time might be the same, so add the "name" to make sure the order is stable - return "commit_time DESC, name ASC" + orderBy = "commit_time DESC, name ASC" + } + if opts.IsDeletedBranch.ValueOrDefault(true) { // if deleted branch included, put them at the beginning + orderBy = "is_deleted ASC, " + orderBy } - return orderBy } From 3e5f85ccf34ca08e95b093211dc4c76e6c866c03 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 31 May 2024 10:30:02 +0800 Subject: [PATCH 10/16] Drop `IDOrderDesc` for listing Actions task and always order by `id DESC` (#31150) Close #31066 Just follow what `FindRunOptions` and `FindScheduleOptions` do. (cherry picked from commit 572fa55fbcc2cb9418b4f7b981a7c80a11899276) --- models/actions/task_list.go | 6 +----- routers/web/shared/actions/runners.go | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/models/actions/task_list.go b/models/actions/task_list.go index 5e17f91441..df4b43c5ef 100644 --- a/models/actions/task_list.go +++ b/models/actions/task_list.go @@ -54,7 +54,6 @@ type FindTaskOptions struct { UpdatedBefore timeutil.TimeStamp StartedBefore timeutil.TimeStamp RunnerID int64 - IDOrderDesc bool } func (opts FindTaskOptions) ToConds() builder.Cond { @@ -84,8 +83,5 @@ func (opts FindTaskOptions) ToConds() builder.Cond { } func (opts FindTaskOptions) ToOrders() string { - if opts.IDOrderDesc { - return "`id` DESC" - } - return "" + return "`id` DESC" } diff --git a/routers/web/shared/actions/runners.go b/routers/web/shared/actions/runners.go index 34b7969442..f38933226b 100644 --- a/routers/web/shared/actions/runners.go +++ b/routers/web/shared/actions/runners.go @@ -79,9 +79,8 @@ func RunnerDetails(ctx *context.Context, page int, runnerID, ownerID, repoID int Page: page, PageSize: 30, }, - Status: actions_model.StatusUnknown, // Unknown means all - IDOrderDesc: true, - RunnerID: runner.ID, + Status: actions_model.StatusUnknown, // Unknown means all + RunnerID: runner.ID, } tasks, count, err := db.FindAndCount[actions_model.ActionTask](ctx, opts) From 47a2102694c47bc30a2a7c673c328471839ef206 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 31 May 2024 20:10:11 +0800 Subject: [PATCH 11/16] Performance improvements for pull request list API (#30490) Fix #30483 --------- Co-authored-by: yp05327 <576951401@qq.com> Co-authored-by: Giteabot (cherry picked from commit 352a2cae247afa254241f113c5c22b9351f116b9) --- models/issues/assignees.go | 12 ++-- models/issues/comment_list.go | 12 ++-- models/issues/issue.go | 96 ++++++++++++++++----------- models/issues/issue_label.go | 6 +- models/issues/issue_list.go | 47 ++++++++------ models/issues/pull.go | 13 ++-- models/issues/pull_list.go | 111 ++++++++++++++++++++++---------- models/user/user.go | 4 ++ routers/api/v1/repo/pull.go | 48 +++++++++----- services/convert/issue.go | 9 ++- services/convert/pull.go | 12 +++- services/issue/assignee_test.go | 3 +- 12 files changed, 243 insertions(+), 130 deletions(-) diff --git a/models/issues/assignees.go b/models/issues/assignees.go index 60f32d9557..a83cb250fa 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -27,23 +27,27 @@ func init() { // LoadAssignees load assignees of this issue. func (issue *Issue) LoadAssignees(ctx context.Context) (err error) { + if issue.isAssigneeLoaded || len(issue.Assignees) > 0 { + return nil + } + // Reset maybe preexisting assignees issue.Assignees = []*user_model.User{} issue.Assignee = nil - err = db.GetEngine(ctx).Table("`user`"). + if err = db.GetEngine(ctx).Table("`user`"). Join("INNER", "issue_assignees", "assignee_id = `user`.id"). Where("issue_assignees.issue_id = ?", issue.ID). - Find(&issue.Assignees) - if err != nil { + Find(&issue.Assignees); err != nil { return err } + issue.isAssigneeLoaded = true // Check if we have at least one assignee and if yes put it in as `Assignee` if len(issue.Assignees) > 0 { issue.Assignee = issue.Assignees[0] } - return err + return nil } // GetAssigneeIDsByIssue returns the IDs of users assigned to an issue diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 370b5396e0..6b4ad80eed 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -16,19 +16,17 @@ import ( // CommentList defines a list of comments type CommentList []*Comment -func (comments CommentList) getPosterIDs() []int64 { - return container.FilterSlice(comments, func(c *Comment) (int64, bool) { - return c.PosterID, c.PosterID > 0 - }) -} - // LoadPosters loads posters func (comments CommentList) LoadPosters(ctx context.Context) error { if len(comments) == 0 { return nil } - posterMaps, err := getPosters(ctx, comments.getPosterIDs()) + posterIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) { + return c.PosterID, c.Poster == nil && c.PosterID > 0 + }) + + posterMaps, err := getPostersByIDs(ctx, posterIDs) if err != nil { return err } diff --git a/models/issues/issue.go b/models/issues/issue.go index 349b7e3d05..f04a4ad5c7 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -98,32 +98,35 @@ var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already // Issue represents an issue or pull request of repository. type Issue struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"` - Repo *repo_model.Repository `xorm:"-"` - Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository. - PosterID int64 `xorm:"INDEX"` - Poster *user_model.User `xorm:"-"` - OriginalAuthor string - OriginalAuthorID int64 `xorm:"index"` - Title string `xorm:"name"` - Content string `xorm:"LONGTEXT"` - RenderedContent template.HTML `xorm:"-"` - ContentVersion int `xorm:"NOT NULL DEFAULT 0"` - Labels []*Label `xorm:"-"` - MilestoneID int64 `xorm:"INDEX"` - Milestone *Milestone `xorm:"-"` - Project *project_model.Project `xorm:"-"` - Priority int - AssigneeID int64 `xorm:"-"` - Assignee *user_model.User `xorm:"-"` - IsClosed bool `xorm:"INDEX"` - IsRead bool `xorm:"-"` - IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not. - PullRequest *PullRequest `xorm:"-"` - NumComments int - Ref string - PinOrder int `xorm:"DEFAULT 0"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"` + Repo *repo_model.Repository `xorm:"-"` + Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository. + PosterID int64 `xorm:"INDEX"` + Poster *user_model.User `xorm:"-"` + OriginalAuthor string + OriginalAuthorID int64 `xorm:"index"` + Title string `xorm:"name"` + Content string `xorm:"LONGTEXT"` + RenderedContent template.HTML `xorm:"-"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` + Labels []*Label `xorm:"-"` + isLabelsLoaded bool `xorm:"-"` + MilestoneID int64 `xorm:"INDEX"` + Milestone *Milestone `xorm:"-"` + isMilestoneLoaded bool `xorm:"-"` + Project *project_model.Project `xorm:"-"` + Priority int + AssigneeID int64 `xorm:"-"` + Assignee *user_model.User `xorm:"-"` + isAssigneeLoaded bool `xorm:"-"` + IsClosed bool `xorm:"INDEX"` + IsRead bool `xorm:"-"` + IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not. + PullRequest *PullRequest `xorm:"-"` + NumComments int + Ref string + PinOrder int `xorm:"DEFAULT 0"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` @@ -134,11 +137,12 @@ type Issue struct { ClosedUnix timeutil.TimeStamp `xorm:"INDEX"` NoAutoTime bool `xorm:"-"` - Attachments []*repo_model.Attachment `xorm:"-"` - Comments CommentList `xorm:"-"` - Reactions ReactionList `xorm:"-"` - TotalTrackedTime int64 `xorm:"-"` - Assignees []*user_model.User `xorm:"-"` + Attachments []*repo_model.Attachment `xorm:"-"` + isAttachmentsLoaded bool `xorm:"-"` + Comments CommentList `xorm:"-"` + Reactions ReactionList `xorm:"-"` + TotalTrackedTime int64 `xorm:"-"` + Assignees []*user_model.User `xorm:"-"` // IsLocked limits commenting abilities to users on an issue // with write access @@ -190,6 +194,19 @@ func (issue *Issue) LoadRepo(ctx context.Context) (err error) { return nil } +func (issue *Issue) LoadAttachments(ctx context.Context) (err error) { + if issue.isAttachmentsLoaded || issue.Attachments != nil { + return nil + } + + issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID) + if err != nil { + return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err) + } + issue.isAttachmentsLoaded = true + return nil +} + // IsTimetrackerEnabled returns true if the repo enables timetracking func (issue *Issue) IsTimetrackerEnabled(ctx context.Context) bool { if err := issue.LoadRepo(ctx); err != nil { @@ -290,11 +307,12 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) { // LoadMilestone load milestone of this issue. func (issue *Issue) LoadMilestone(ctx context.Context) (err error) { - if (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 { + if !issue.isMilestoneLoaded && (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 { issue.Milestone, err = GetMilestoneByRepoID(ctx, issue.RepoID, issue.MilestoneID) if err != nil && !IsErrMilestoneNotExist(err) { return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %w", issue.RepoID, issue.MilestoneID, err) } + issue.isMilestoneLoaded = true } return nil } @@ -330,11 +348,8 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { return err } - if issue.Attachments == nil { - issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID) - if err != nil { - return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err) - } + if err = issue.LoadAttachments(ctx); err != nil { + return err } if err = issue.loadComments(ctx); err != nil { @@ -353,6 +368,13 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { return issue.loadReactions(ctx) } +func (issue *Issue) ResetAttributesLoaded() { + issue.isLabelsLoaded = false + issue.isMilestoneLoaded = false + issue.isAttachmentsLoaded = false + issue.isAssigneeLoaded = false +} + // GetIsRead load the `IsRead` field of the issue func (issue *Issue) GetIsRead(ctx context.Context, userID int64) error { issueUser := &IssueUser{IssueID: issue.ID, UID: userID} diff --git a/models/issues/issue_label.go b/models/issues/issue_label.go index 733f1043b0..10fc821454 100644 --- a/models/issues/issue_label.go +++ b/models/issues/issue_label.go @@ -111,6 +111,7 @@ func NewIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m return err } + issue.isLabelsLoaded = false issue.Labels = nil if err = issue.LoadLabels(ctx); err != nil { return err @@ -160,6 +161,8 @@ func NewIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *us return err } + // reload all labels + issue.isLabelsLoaded = false issue.Labels = nil if err = issue.LoadLabels(ctx); err != nil { return err @@ -325,11 +328,12 @@ func FixIssueLabelWithOutsideLabels(ctx context.Context) (int64, error) { // LoadLabels loads labels func (issue *Issue) LoadLabels(ctx context.Context) (err error) { - if issue.Labels == nil && issue.ID != 0 { + if !issue.isLabelsLoaded && issue.Labels == nil && issue.ID != 0 { issue.Labels, err = GetLabelsByIssueID(ctx, issue.ID) if err != nil { return fmt.Errorf("getLabelsByIssueID [%d]: %w", issue.ID, err) } + issue.isLabelsLoaded = true } return nil } diff --git a/models/issues/issue_list.go b/models/issues/issue_list.go index 2235f3d3a8..ea6753c6f5 100644 --- a/models/issues/issue_list.go +++ b/models/issues/issue_list.go @@ -73,18 +73,16 @@ func (issues IssueList) LoadRepositories(ctx context.Context) (repo_model.Reposi return repo_model.ValuesRepository(repoMaps), nil } -func (issues IssueList) getPosterIDs() []int64 { - return container.FilterSlice(issues, func(issue *Issue) (int64, bool) { - return issue.PosterID, true - }) -} - -func (issues IssueList) loadPosters(ctx context.Context) error { +func (issues IssueList) LoadPosters(ctx context.Context) error { if len(issues) == 0 { return nil } - posterMaps, err := getPosters(ctx, issues.getPosterIDs()) + posterIDs := container.FilterSlice(issues, func(issue *Issue) (int64, bool) { + return issue.PosterID, issue.Poster == nil && issue.PosterID > 0 + }) + + posterMaps, err := getPostersByIDs(ctx, posterIDs) if err != nil { return err } @@ -95,7 +93,7 @@ func (issues IssueList) loadPosters(ctx context.Context) error { return nil } -func getPosters(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) { +func getPostersByIDs(ctx context.Context, posterIDs []int64) (map[int64]*user_model.User, error) { posterMaps := make(map[int64]*user_model.User, len(posterIDs)) left := len(posterIDs) for left > 0 { @@ -137,7 +135,7 @@ func (issues IssueList) getIssueIDs() []int64 { return ids } -func (issues IssueList) loadLabels(ctx context.Context) error { +func (issues IssueList) LoadLabels(ctx context.Context) error { if len(issues) == 0 { return nil } @@ -169,7 +167,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error { err = rows.Scan(&labelIssue) if err != nil { if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadLabels: Close: %w", err1) + return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1) } return err } @@ -178,7 +176,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error { // When there are no rows left and we try to close it. // Since that is not relevant for us, we can safely ignore it. if err1 := rows.Close(); err1 != nil { - return fmt.Errorf("IssueList.loadLabels: Close: %w", err1) + return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1) } left -= limit issueIDs = issueIDs[limit:] @@ -186,6 +184,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error { for _, issue := range issues { issue.Labels = issueLabels[issue.ID] + issue.isLabelsLoaded = true } return nil } @@ -196,7 +195,7 @@ func (issues IssueList) getMilestoneIDs() []int64 { }) } -func (issues IssueList) loadMilestones(ctx context.Context) error { +func (issues IssueList) LoadMilestones(ctx context.Context) error { milestoneIDs := issues.getMilestoneIDs() if len(milestoneIDs) == 0 { return nil @@ -221,6 +220,7 @@ func (issues IssueList) loadMilestones(ctx context.Context) error { for _, issue := range issues { issue.Milestone = milestoneMaps[issue.MilestoneID] + issue.isMilestoneLoaded = true } return nil } @@ -264,7 +264,7 @@ func (issues IssueList) LoadProjects(ctx context.Context) error { return nil } -func (issues IssueList) loadAssignees(ctx context.Context) error { +func (issues IssueList) LoadAssignees(ctx context.Context) error { if len(issues) == 0 { return nil } @@ -311,6 +311,10 @@ func (issues IssueList) loadAssignees(ctx context.Context) error { for _, issue := range issues { issue.Assignees = assignees[issue.ID] + if len(issue.Assignees) > 0 { + issue.Assignee = issue.Assignees[0] + } + issue.isAssigneeLoaded = true } return nil } @@ -414,6 +418,7 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) { for _, issue := range issues { issue.Attachments = attachments[issue.ID] + issue.isAttachmentsLoaded = true } return nil } @@ -539,23 +544,23 @@ func (issues IssueList) LoadAttributes(ctx context.Context) error { return fmt.Errorf("issue.loadAttributes: LoadRepositories: %w", err) } - if err := issues.loadPosters(ctx); err != nil { - return fmt.Errorf("issue.loadAttributes: loadPosters: %w", err) + if err := issues.LoadPosters(ctx); err != nil { + return fmt.Errorf("issue.loadAttributes: LoadPosters: %w", err) } - if err := issues.loadLabels(ctx); err != nil { - return fmt.Errorf("issue.loadAttributes: loadLabels: %w", err) + if err := issues.LoadLabels(ctx); err != nil { + return fmt.Errorf("issue.loadAttributes: LoadLabels: %w", err) } - if err := issues.loadMilestones(ctx); err != nil { - return fmt.Errorf("issue.loadAttributes: loadMilestones: %w", err) + if err := issues.LoadMilestones(ctx); err != nil { + return fmt.Errorf("issue.loadAttributes: LoadMilestones: %w", err) } if err := issues.LoadProjects(ctx); err != nil { return fmt.Errorf("issue.loadAttributes: loadProjects: %w", err) } - if err := issues.loadAssignees(ctx); err != nil { + if err := issues.LoadAssignees(ctx); err != nil { return fmt.Errorf("issue.loadAttributes: loadAssignees: %w", err) } diff --git a/models/issues/pull.go b/models/issues/pull.go index 4194df2e3d..ae7fb35eab 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -159,10 +159,11 @@ type PullRequest struct { ChangedProtectedFiles []string `xorm:"TEXT JSON"` - IssueID int64 `xorm:"INDEX"` - Issue *Issue `xorm:"-"` - Index int64 - RequestedReviewers []*user_model.User `xorm:"-"` + IssueID int64 `xorm:"INDEX"` + Issue *Issue `xorm:"-"` + Index int64 + RequestedReviewers []*user_model.User `xorm:"-"` + isRequestedReviewersLoaded bool `xorm:"-"` HeadRepoID int64 `xorm:"INDEX"` HeadRepo *repo_model.Repository `xorm:"-"` @@ -289,7 +290,7 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { // LoadRequestedReviewers loads the requested reviewers. func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error { - if len(pr.RequestedReviewers) > 0 { + if pr.isRequestedReviewersLoaded || len(pr.RequestedReviewers) > 0 { return nil } @@ -297,10 +298,10 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error { if err != nil { return err } - if err = reviews.LoadReviewers(ctx); err != nil { return err } + pr.isRequestedReviewersLoaded = true for _, review := range reviews { pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer) } diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index 61b4168ea2..52f2f01a5a 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -9,8 +9,10 @@ import ( "code.gitea.io/gitea/models/db" access_model "code.gitea.io/gitea/models/perm/access" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" @@ -129,7 +131,7 @@ func GetPullRequestIDsByCheckStatus(ctx context.Context, status PullRequestStatu } // PullRequests returns all pull requests for a base Repo by the given conditions -func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptions) ([]*PullRequest, int64, error) { +func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptions) (PullRequestList, int64, error) { if opts.Page <= 0 { opts.Page = 1 } @@ -159,50 +161,93 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio // PullRequestList defines a list of pull requests type PullRequestList []*PullRequest -func (prs PullRequestList) LoadAttributes(ctx context.Context) error { - if len(prs) == 0 { - return nil +func (prs PullRequestList) getRepositoryIDs() []int64 { + repoIDs := make(container.Set[int64]) + for _, pr := range prs { + if pr.BaseRepo == nil && pr.BaseRepoID > 0 { + repoIDs.Add(pr.BaseRepoID) + } + if pr.HeadRepo == nil && pr.HeadRepoID > 0 { + repoIDs.Add(pr.HeadRepoID) + } } + return repoIDs.Values() +} - // Load issues. - issueIDs := prs.GetIssueIDs() - issues := make([]*Issue, 0, len(issueIDs)) +func (prs PullRequestList) LoadRepositories(ctx context.Context) error { + repoIDs := prs.getRepositoryIDs() + reposMap := make(map[int64]*repo_model.Repository, len(repoIDs)) if err := db.GetEngine(ctx). - Where("id > 0"). - In("id", issueIDs). - Find(&issues); err != nil { - return fmt.Errorf("find issues: %w", err) - } - - set := make(map[int64]*Issue) - for i := range issues { - set[issues[i].ID] = issues[i] + In("id", repoIDs). + Find(&reposMap); err != nil { + return fmt.Errorf("find repos: %w", err) } for _, pr := range prs { - pr.Issue = set[pr.IssueID] - /* - Old code: - pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync - - It's worth panic because it's almost impossible to happen under normal use. - But in integration testing, an asynchronous task could read a database that has been reset. - So returning an error would make more sense, let the caller has a choice to ignore it. - */ - if pr.Issue == nil { - return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist) + if pr.BaseRepo == nil { + pr.BaseRepo = reposMap[pr.BaseRepoID] + } + if pr.HeadRepo == nil { + pr.HeadRepo = reposMap[pr.HeadRepoID] + pr.isHeadRepoLoaded = true } - pr.Issue.PullRequest = pr } return nil } +func (prs PullRequestList) LoadAttributes(ctx context.Context) error { + if _, err := prs.LoadIssues(ctx); err != nil { + return err + } + return nil +} + +func (prs PullRequestList) LoadIssues(ctx context.Context) (IssueList, error) { + if len(prs) == 0 { + return nil, nil + } + + // Load issues. + issueIDs := prs.GetIssueIDs() + issues := make(map[int64]*Issue, len(issueIDs)) + if err := db.GetEngine(ctx). + In("id", issueIDs). + Find(&issues); err != nil { + return nil, fmt.Errorf("find issues: %w", err) + } + + issueList := make(IssueList, 0, len(prs)) + for _, pr := range prs { + if pr.Issue == nil { + pr.Issue = issues[pr.IssueID] + /* + Old code: + pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync + + It's worth panic because it's almost impossible to happen under normal use. + But in integration testing, an asynchronous task could read a database that has been reset. + So returning an error would make more sense, let the caller has a choice to ignore it. + */ + if pr.Issue == nil { + return nil, fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist) + } + } + pr.Issue.PullRequest = pr + if pr.Issue.Repo == nil { + pr.Issue.Repo = pr.BaseRepo + } + issueList = append(issueList, pr.Issue) + } + return issueList, nil +} + // GetIssueIDs returns all issue ids func (prs PullRequestList) GetIssueIDs() []int64 { - issueIDs := make([]int64, 0, len(prs)) - for i := range prs { - issueIDs = append(issueIDs, prs[i].IssueID) - } - return issueIDs + return container.FilterSlice(prs, func(pr *PullRequest) (int64, bool) { + if pr.Issue == nil { + return pr.IssueID, pr.IssueID > 0 + } + return 0, false + }) } // HasMergedPullRequestInRepo returns whether the user(poster) has merged pull-request in the repo diff --git a/models/user/user.go b/models/user/user.go index 58808c71b9..6d8c5fa2b5 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -894,6 +894,10 @@ func GetUserByID(ctx context.Context, id int64) (*User, error) { // GetUserByIDs returns the user objects by given IDs if exists. func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) { + if len(ids) == 0 { + return nil, nil + } + users := make([]*User, 0, len(ids)) err := db.GetEngine(ctx).In("id", ids). Table("user"). diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 1a326494f2..b8314d6eb0 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -116,23 +116,39 @@ func ListPullRequests(ctx *context.APIContext) { } apiPrs := make([]*api.PullRequest, len(prs)) + // NOTE: load repository first, so that issue.Repo will be filled with pr.BaseRepo + if err := prs.LoadRepositories(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadRepositories", err) + return + } + issueList, err := prs.LoadIssues(ctx) + if err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssues", err) + return + } + + if err := issueList.LoadLabels(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadLabels", err) + return + } + if err := issueList.LoadPosters(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadPoster", err) + return + } + if err := issueList.LoadAttachments(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) + return + } + if err := issueList.LoadMilestones(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadMilestones", err) + return + } + if err := issueList.LoadAssignees(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAssignees", err) + return + } + for i := range prs { - if err = prs[i].LoadIssue(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, "LoadIssue", err) - return - } - if err = prs[i].LoadAttributes(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) - return - } - if err = prs[i].LoadBaseRepo(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, "LoadBaseRepo", err) - return - } - if err = prs[i].LoadHeadRepo(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err) - return - } apiPrs[i] = convert.ToAPIPullRequest(ctx, prs[i], ctx.Doer) } diff --git a/services/convert/issue.go b/services/convert/issue.go index 668affe09a..047feb8aa7 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -31,15 +31,15 @@ func ToAPIIssue(ctx context.Context, doer *user_model.User, issue *issues_model. } func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, getDownloadURL func(repo *repo_model.Repository, attach *repo_model.Attachment) string) *api.Issue { - if err := issue.LoadLabels(ctx); err != nil { - return &api.Issue{} - } if err := issue.LoadPoster(ctx); err != nil { return &api.Issue{} } if err := issue.LoadRepo(ctx); err != nil { return &api.Issue{} } + if err := issue.LoadAttachments(ctx); err != nil { + return &api.Issue{} + } apiIssue := &api.Issue{ ID: issue.ID, @@ -63,6 +63,9 @@ func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Iss } apiIssue.URL = issue.APIURL(ctx) apiIssue.HTMLURL = issue.HTMLURL() + if err := issue.LoadLabels(ctx); err != nil { + return &api.Issue{} + } apiIssue.Labels = ToLabelList(issue.Labels, issue.Repo, issue.Repo.Owner) apiIssue.Repo = &api.RepositoryMeta{ ID: issue.Repo.ID, diff --git a/services/convert/pull.go b/services/convert/pull.go index 775bf3806d..da75cb3db2 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -44,7 +45,16 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u return nil } - p, err := access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer) + var doerID int64 + if doer != nil { + doerID = doer.ID + } + + const repoDoerPermCacheKey = "repo_doer_perm_cache" + p, err := cache.GetWithContextCache(ctx, repoDoerPermCacheKey, fmt.Sprintf("%d_%d", pr.BaseRepoID, doerID), + func() (access_model.Permission, error) { + return access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer) + }) if err != nil { log.Error("GetUserRepoPermission[%d]: %v", pr.BaseRepoID, err) p.AccessMode = perm.AccessModeNone diff --git a/services/issue/assignee_test.go b/services/issue/assignee_test.go index da25da60ee..38d56f9d9d 100644 --- a/services/issue/assignee_test.go +++ b/services/issue/assignee_test.go @@ -39,7 +39,8 @@ func TestDeleteNotPassedAssignee(t *testing.T) { assert.NoError(t, err) assert.Empty(t, issue.Assignees) - // Check they're gone + // Reload to check they're gone + issue.ResetAttributesLoaded() assert.NoError(t, issue.LoadAssignees(db.DefaultContext)) assert.Empty(t, issue.Assignees) assert.Empty(t, issue.Assignee) From 737c3e90eb0838bd14ad236d27400b3748684810 Mon Sep 17 00:00:00 2001 From: Max Wipfli Date: Sun, 2 Jun 2024 04:32:20 +0200 Subject: [PATCH 12/16] Only update poster in issue/comment list if it has been loaded (#31216) Previously, all posters were updated, even if they were not part of posterMaps. In that case, a ghost user was erroneously inserted. Fixes #31213. (cherry picked from commit 3cc7f763c3c22ae4c3b5331f8b72b7009c5b11ea) --- models/issues/comment_list.go | 4 +++- models/issues/issue_list.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 6b4ad80eed..61ac1c8f56 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -32,7 +32,9 @@ func (comments CommentList) LoadPosters(ctx context.Context) error { } for _, comment := range comments { - comment.Poster = getPoster(comment.PosterID, posterMaps) + if comment.Poster == nil { + comment.Poster = getPoster(comment.PosterID, posterMaps) + } } return nil } diff --git a/models/issues/issue_list.go b/models/issues/issue_list.go index ea6753c6f5..fbfa7584a0 100644 --- a/models/issues/issue_list.go +++ b/models/issues/issue_list.go @@ -88,7 +88,9 @@ func (issues IssueList) LoadPosters(ctx context.Context) error { } for _, issue := range issues { - issue.Poster = getPoster(issue.PosterID, posterMaps) + if issue.Poster == nil { + issue.Poster = getPoster(issue.PosterID, posterMaps) + } } return nil } From f6e2b79dd8ac59e69c7ef1eacaba87b1b6360c26 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Jun 2024 11:01:08 +0800 Subject: [PATCH 13/16] Fix the possible migration failure on 286 with postgres 16 (#31209) Try to fix #31205 (cherry picked from commit 98a61040b1c83790b0e0e977188842f967ae357e) --- models/migrations/v1_22/v286.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v1_22/v286.go b/models/migrations/v1_22/v286.go index 2d62d7788c..6a5f45122a 100644 --- a/models/migrations/v1_22/v286.go +++ b/models/migrations/v1_22/v286.go @@ -60,7 +60,7 @@ func addObjectFormatNameToRepository(x *xorm.Engine) error { // Here to catch weird edge-cases where column constraints above are // not applied by the DB backend - _, err := x.Exec("UPDATE repository set object_format_name = 'sha1' WHERE object_format_name = '' or object_format_name IS NULL") + _, err := x.Exec("UPDATE `repository` set `object_format_name` = 'sha1' WHERE `object_format_name` = '' or `object_format_name` IS NULL") return err } From b4887fcd3788d86a15495d2295991c29db1d5c03 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 2 Jun 2024 12:46:54 +0200 Subject: [PATCH 14/16] chore(deadcode): update --- .deadcode-out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.deadcode-out b/.deadcode-out index ac62e77ba7..a6714d76a9 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -77,7 +77,7 @@ package "code.gitea.io/gitea/models/perm/access" func GetRepoWriters package "code.gitea.io/gitea/models/project" - func UpdateBoardSorting + func UpdateColumnSorting func ChangeProjectStatus package "code.gitea.io/gitea/models/repo" From 62448bfb931882859388b2fd472cb89428c25323 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 29 May 2024 14:43:02 +0800 Subject: [PATCH 15/16] Fix push multiple branches error with tests (#31151) (cherry picked from commit 5c1b550e00e9460078e00c41a32d206b260ef482) Conflicts: tests/integration/git_push_test.go trivial context conflict because of 2ac3dcbd43 test: hook post-receive for sha256 repos --- services/repository/branch.go | 2 +- .../git_helper_for_declarative_test.go | 18 ++++++++++ tests/integration/git_push_test.go | 35 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/services/repository/branch.go b/services/repository/branch.go index b34bfa5fd5..27e50e5ced 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -296,7 +296,7 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, branchNames, if _, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit); err != nil { return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err) } - return nil + continue } // if database have branches but not this branch, it means this is a new branch diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index ff06dab07a..7392c0eb13 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -161,6 +161,24 @@ func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T } } +func doGitAddSomeCommits(dstPath, branch string) func(*testing.T) { + return func(t *testing.T) { + doGitCheckoutBranch(dstPath, branch)(t) + + assert.NoError(t, os.WriteFile(filepath.Join(dstPath, fmt.Sprintf("file-%s.txt", branch)), []byte(fmt.Sprintf("file %s", branch)), 0o644)) + assert.NoError(t, git.AddChanges(dstPath, true)) + signature := git.Signature{ + Email: "test@test.test", + Name: "test", + } + assert.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &signature, + Author: &signature, + Message: fmt.Sprintf("update %s", branch), + })) + } +} + func doGitCreateBranch(dstPath, branch string) func(*testing.T) { return func(t *testing.T) { _, _, err := git.NewCommand(git.DefaultContext, "checkout", "-b").AddDynamicArguments(branch).RunStdString(&git.RunOpts{Dir: dstPath}) diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index bbc80cc927..8a9fe81e24 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -51,6 +51,41 @@ func testGitPush(t *testing.T, u *url.URL) { }) }) + t.Run("Push branches exists", func(t *testing.T) { + runTestGitPush(t, u, objectFormat, func(t *testing.T, gitPath string) (pushed, deleted []string) { + for i := 0; i < 10; i++ { + branchName := fmt.Sprintf("branch-%d", i) + if i < 5 { + pushed = append(pushed, branchName) + } + doGitCreateBranch(gitPath, branchName)(t) + } + // only push master and the first 5 branches + pushed = append(pushed, "master") + args := append([]string{"origin"}, pushed...) + doGitPushTestRepository(gitPath, args...)(t) + + pushed = pushed[:0] + // do some changes for the first 5 branches created above + for i := 0; i < 5; i++ { + branchName := fmt.Sprintf("branch-%d", i) + pushed = append(pushed, branchName) + + doGitAddSomeCommits(gitPath, branchName)(t) + } + + for i := 5; i < 10; i++ { + pushed = append(pushed, fmt.Sprintf("branch-%d", i)) + } + pushed = append(pushed, "master") + + // push all, so that master are not chagned + doGitPushTestRepository(gitPath, "origin", "--all")(t) + + return pushed, deleted + }) + }) + t.Run("Push branches one by one", func(t *testing.T) { runTestGitPush(t, u, objectFormat, func(t *testing.T, gitPath string) (pushed, deleted []string) { for i := 0; i < 100; i++ { From 7e8890cc55672e0571c18a1eb84787cd8c0fdfbd Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Mon, 3 Jun 2024 07:43:44 +0200 Subject: [PATCH 16/16] docs(release): add release notes --- release-notes/7.0.4/fix/4004.md | 1 + release-notes/8.0.0/feat/3989.md | 4 ++++ release-notes/8.0.0/perf/3989.md | 1 + 3 files changed, 6 insertions(+) create mode 100644 release-notes/7.0.4/fix/4004.md create mode 100644 release-notes/8.0.0/feat/3989.md create mode 100644 release-notes/8.0.0/perf/3989.md diff --git a/release-notes/7.0.4/fix/4004.md b/release-notes/7.0.4/fix/4004.md new file mode 100644 index 0000000000..3e8bfcb7d0 --- /dev/null +++ b/release-notes/7.0.4/fix/4004.md @@ -0,0 +1 @@ +- "Git hooks of this repository seem to be broken." [warning when pushing more than one branch at a time](https://codeberg.org/forgejo/forgejo/commit/62448bfb931882859388b2fd472cb89428c25323) diff --git a/release-notes/8.0.0/feat/3989.md b/release-notes/8.0.0/feat/3989.md new file mode 100644 index 0000000000..228c4e9bce --- /dev/null +++ b/release-notes/8.0.0/feat/3989.md @@ -0,0 +1,4 @@ +- API endpoints that return a repository now [also include the topics](https://codeberg.org/forgejo/forgejo/commit/ee2247d77c0b13b0b45df704d7589b541db03899) +- Display an error when an issue comment is [edited simultaneously by two users](https://codeberg.org/forgejo/forgejo/commit/ca0921a95aa9a37d8820538458c15fd0a3b0c97c) instead of silently overriding one of them +- Add [support for a credentials chain for minio](https://codeberg.org/forgejo/forgejo/commit/73706ae26d138684ef9da9e1164846a040fd4a7d) +- [Rename project board into column](https://codeberg.org/forgejo/forgejo/commit/a7591f9738dbefb2dcddeb2d45175abee3d03c1f) because it was confusing to users diff --git a/release-notes/8.0.0/perf/3989.md b/release-notes/8.0.0/perf/3989.md new file mode 100644 index 0000000000..d3d77695e2 --- /dev/null +++ b/release-notes/8.0.0/perf/3989.md @@ -0,0 +1 @@ +- improve performances when [retrieving pull requests via the API](https://codeberg.org/forgejo/forgejo/commit/47a2102694c47bc30a2a7c673c328471839ef206)