From 632fb5badcbe1636e7cf25ad6e4f176fabf47677 Mon Sep 17 00:00:00 2001 From: Shashvat Kedia Date: Wed, 8 Jan 2020 17:23:07 +0530 Subject: [PATCH 1/9] Fix #9552: Merge commits generated by pull request capture pull request details (#9635) --- models/pull.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index f86892cbfc..876036c9e1 100644 --- a/models/pull.go +++ b/models/pull.go @@ -175,7 +175,11 @@ func (pr *PullRequest) GetDefaultMergeMessage() string { return "" } } - return fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.MustHeadUserName(), pr.HeadRepo.Name, pr.BaseBranch) + if err := pr.LoadIssue(); err != nil { + log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err) + return "" + } + return fmt.Sprintf("Merge pull request '%s' (#%d) from %s/%s into %s", pr.Issue.Title, pr.Issue.Index, pr.MustHeadUserName(), pr.HeadBranch, pr.BaseBranch) } // GetCommitMessages returns the commit messages between head and merge base (if there is one) From 98772d376c2a140d100f6ed9b09bfb63a3b817ca Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Wed, 8 Jan 2020 12:45:05 +0000 Subject: [PATCH 2/9] [skip ci] Updated translations via Crowdin --- options/locale/locale_lv-LV.ini | 48 ++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_lv-LV.ini b/options/locale/locale_lv-LV.ini index d05e672d09..6a59492de4 100644 --- a/options/locale/locale_lv-LV.ini +++ b/options/locale/locale_lv-LV.ini @@ -1596,7 +1596,7 @@ settings.full_name=Pilns vārds, uzvārds settings.website=Mājas lapa settings.location=Atrašanās vieta settings.permission=Tiesības -settings.repoadminchangeteam=Repozitorija administrators var pievienot vain noņemt piekļuvi komandām +settings.repoadminchangeteam=Repozitorija administrators var pievienot vai noņemt piekļuvi komandām settings.visibility=Redzamība settings.visibility.public=Publiska settings.visibility.limited=Ierobežota (redzama tikai autorizētiem lietotājiem) @@ -2025,8 +2025,54 @@ monitor.execute_time=Izpildes laiks monitor.process.cancel=Atcelt procesu monitor.process.cancel_desc=Procesa atcelšana var radīt datu zaudējumus monitor.process.cancel_notices=Atcelt: %s? +monitor.queues=Rindas +monitor.queue=Rinda: %s +monitor.queue.name=Nosaukums +monitor.queue.type=Veids +monitor.queue.exemplar=Eksemplāra veids +monitor.queue.numberworkers=Strādņu skaits +monitor.queue.maxnumberworkers=Maksimālais strādņu skaits +monitor.queue.review=Pārbaudīt konfigurāciju +monitor.queue.review_add=Pārbaudīt/Pievienot strādņus +monitor.queue.configuration=Sākotnējā konfigurācija +monitor.queue.nopool.title=Nav strādņu pūla +monitor.queue.nopool.desc=Šī rinda apvieno citas rindas un tai nav strādņu pūla. +monitor.queue.wrapped.desc=Apvienojošā rinda apvieno lēni startējošās rindas, uzkrājot sarindotos pieprasījumus kanālā. Tai nav strādņu pūla. +monitor.queue.persistable-channel.desc=Patstāvīgas kanāli apvieno divas rindas, kanāla rindu, kurai ir savs strādņu pūls un līmeņu rindu patstāvīgajiem pieprasījumiem no iepriekšejām izslēgšanām. Tai nav strādņu pūla. +monitor.queue.pool.timeout=Noildze +monitor.queue.pool.addworkers.title=Pievienot strādņus +monitor.queue.pool.addworkers.submit=Pievienot +monitor.queue.pool.addworkers.desc=Pievienot strādņus šim pūlam ar vai bez noildzes. Ja uzstādīsies noildzi, tad šie strādņi tiks noņemti no pūla, kad noildze būs iestājusies. +monitor.queue.pool.addworkers.numberworkers.placeholder=Strādņu skaits +monitor.queue.pool.addworkers.timeout.placeholder=Norādiet 0, lai nebūtu noildzes +monitor.queue.pool.addworkers.mustnumbergreaterzero=Strādņu skaitam, ko pievienot, ir jābūt lielākam par nulli +monitor.queue.pool.addworkers.musttimeoutduration=Noildzei ir jābūt norādītai kā ilgumam, piemēram, 5m vai 0 +monitor.queue.settings.title=Pūla iestatījumi +monitor.queue.settings.desc=Pūli var dinamiski augt un paildzinātu atbildi uz strādņu rindas bloķēšanu. Šis izmaiņas ietekmēs pašreizējās strādņu grupas. +monitor.queue.settings.timeout=Pagarināt noildzi +monitor.queue.settings.timeout.placeholder=Pašalaik %[1]v +monitor.queue.settings.timeout.error=Noildzei ir jābūt norādītai kā ilgumam, piemēram, 5m vai 0 +monitor.queue.settings.numberworkers=Palielināt strādņu skaitu +monitor.queue.settings.numberworkers.placeholder=Pašalaik %[1]d +monitor.queue.settings.numberworkers.error=Strādņu skaitam ir jābūt lielākam vai vienādam ar nulli +monitor.queue.settings.maxnumberworkers=Maksimālais strādņu skaits +monitor.queue.settings.maxnumberworkers.placeholder=Pašalaik %[1]d +monitor.queue.settings.maxnumberworkers.error=Maksimālajam strādņu skaitam ir jābūt skaitlim +monitor.queue.settings.submit=Saglabāt iestatījumus +monitor.queue.settings.changed=Iestatījumi saglabāti +monitor.queue.settings.blocktimeout=Pašreizējās grupas noildze +monitor.queue.settings.blocktimeout.value=%[1]v +monitor.queue.pool.none=Rindai nav pūla +monitor.queue.pool.added=Strādņu grupa pievienota +monitor.queue.pool.max_changed=Maksimālais strādņu skaits mainīts +monitor.queue.pool.workers.title=Aktīvās strādņu grupas +monitor.queue.pool.workers.none=Nav strādņu grupu. +monitor.queue.pool.cancel=Izslēgt strādņu grupu +monitor.queue.pool.cancelling=Strādņu grupa tiek izslēgta +monitor.queue.pool.cancel_notices=Izslēgt šo grupu ar %s strādņiem? +monitor.queue.pool.cancel_desc=Atstājot rindu bez nevienas strādņu grupas, var radīt pieprasījumu bloķēšanos. notices.system_notice_list=Sistēmas paziņojumi notices.view_detail_header=Skatīt paziņojuma detaļas From c779ac12c971a4347d085f8dbca7531faab16221 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 8 Jan 2020 15:30:58 +0100 Subject: [PATCH 3/9] fix #9648 | use filepath.IsAbs instead of path.IsAbs (#9651) * fix #9648 * found next Co-authored-by: Antoine GIRARD --- modules/setting/queue.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/setting/queue.go b/modules/setting/queue.go index 546802715f..c91ff55acd 100644 --- a/modules/setting/queue.go +++ b/modules/setting/queue.go @@ -7,6 +7,7 @@ package setting import ( "fmt" "path" + "path/filepath" "strconv" "strings" "time" @@ -44,7 +45,7 @@ func GetQueueSettings(name string) QueueSettings { q := QueueSettings{} sec := Cfg.Section("queue." + name) // DataDir is not directly inheritable - q.DataDir = path.Join(Queue.DataDir, name) + q.DataDir = filepath.Join(Queue.DataDir, name) // QueueName is not directly inheritable either q.QueueName = name + Queue.QueueName for _, key := range sec.Keys() { @@ -55,8 +56,8 @@ func GetQueueSettings(name string) QueueSettings { q.QueueName = key.MustString(q.QueueName) } } - if !path.IsAbs(q.DataDir) { - q.DataDir = path.Join(AppDataPath, q.DataDir) + if !filepath.IsAbs(q.DataDir) { + q.DataDir = filepath.Join(AppDataPath, q.DataDir) } sec.Key("DATADIR").SetValue(q.DataDir) // The rest are... @@ -82,8 +83,8 @@ func GetQueueSettings(name string) QueueSettings { func NewQueueService() { sec := Cfg.Section("queue") Queue.DataDir = sec.Key("DATADIR").MustString("queues/") - if !path.IsAbs(Queue.DataDir) { - Queue.DataDir = path.Join(AppDataPath, Queue.DataDir) + if !filepath.IsAbs(Queue.DataDir) { + Queue.DataDir = filepath.Join(AppDataPath, Queue.DataDir) } Queue.Length = sec.Key("LENGTH").MustInt(20) Queue.BatchLength = sec.Key("BATCH_LENGTH").MustInt(20) From b822518e396a569b89aab6d621b01eefe723caa7 Mon Sep 17 00:00:00 2001 From: Bagas Sanjaya Date: Wed, 8 Jan 2020 23:33:13 +0700 Subject: [PATCH 4/9] [Docs] Linux Service Edit (#9633) * Rename h3 title * Add intro Should work on Ubuntu Xenial, but should work on any Linux distros. * Indirect edit files Instead of providing `sudo vim`, invite to edit files. * enable now instead of enable and start * Re-add systemctl enable && systemctl start * Revert service enablement back to status quo * Add enable now counterpart for systemd > v220 * Apply suggestions from @sapk Strip `vim` from editor usage Co-Authored-By: Antoine GIRARD Co-authored-by: Antoine GIRARD --- .../run-as-service-in-ubuntu.en-us.md | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/content/doc/installation/run-as-service-in-ubuntu.en-us.md b/docs/content/doc/installation/run-as-service-in-ubuntu.en-us.md index d3b5df5b5a..7af062c3aa 100644 --- a/docs/content/doc/installation/run-as-service-in-ubuntu.en-us.md +++ b/docs/content/doc/installation/run-as-service-in-ubuntu.en-us.md @@ -13,16 +13,13 @@ menu: identifier: "linux-service" --- -### Run as service in Ubuntu 16.04 LTS +### Run Gitea as Linux service + +You can run Gitea as service, using either systemd or supervisor. The steps below tested on Ubuntu 16.04, but those should work on any Linux distributions (with little modification). #### Using systemd -Run the below command in a terminal: -``` -sudo vim /etc/systemd/system/gitea.service -``` - -Copy the sample [gitea.service](https://github.com/go-gitea/gitea/blob/master/contrib/systemd/gitea.service). +Copy the sample [gitea.service](https://github.com/go-gitea/gitea/blob/master/contrib/systemd/gitea.service) to `/etc/systemd/system/gitea.service`, then edit the file with your favorite editor. Uncomment any service that needs to be enabled on this host, such as MySQL. @@ -35,6 +32,10 @@ sudo systemctl enable gitea sudo systemctl start gitea ``` +If you have systemd version 220 or later, you can enable and immediately start Gitea at once by: +``` +sudo systemctl enable gitea --now +``` #### Using supervisor @@ -49,19 +50,20 @@ Create a log dir for the supervisor logs: mkdir /home/git/gitea/log/supervisor ``` -Open supervisor config file in a file editor: -``` -sudo vim /etc/supervisor/supervisord.conf -``` - Append the configuration from the sample -[supervisord config](https://github.com/go-gitea/gitea/blob/master/contrib/supervisor/gitea). +[supervisord config](https://github.com/go-gitea/gitea/blob/master/contrib/supervisor/gitea) to `/etc/supervisor/supervisord.conf`. -Change the user (git) and home (/home/git) settings to match the deployment -environment. Change the PORT or remove the -p flag if default port is used. +Using your favorite editor, change the user (git) and home +(/home/git) settings to match the deployment environment. Change the PORT +or remove the -p flag if default port is used. Lastly enable and start supervisor at boot: ``` sudo systemctl enable supervisor sudo systemctl start supervisor ``` + +If you have systemd version 220 or later, you can enable and immediately start supervisor by: +``` +sudo systemctl enable supervisor --now +``` From f8dcc5f9f8e389218f1908ad9d5fe2044102abf1 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Wed, 8 Jan 2020 11:45:24 -0600 Subject: [PATCH 5/9] Add PR review webhook to Telegram (#9653) Signed-off-by: jolheiser Co-authored-by: Antoine GIRARD --- modules/webhook/telegram.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/modules/webhook/telegram.go b/modules/webhook/telegram.go index 42adb40be2..47d54f7cb9 100644 --- a/modules/webhook/telegram.go +++ b/modules/webhook/telegram.go @@ -148,6 +148,25 @@ func getTelegramPullRequestPayload(p *api.PullRequestPayload) (*TelegramPayload, }, nil } +func getTelegramPullRequestApprovalPayload(p *api.PullRequestPayload, event models.HookEventType) (*TelegramPayload, error) { + var text, attachmentText string + switch p.Action { + case api.HookIssueSynchronized: + action, err := parseHookPullRequestEventType(event) + if err != nil { + return nil, err + } + + text = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title) + attachmentText = p.Review.Content + + } + + return &TelegramPayload{ + Message: text + "\n" + attachmentText, + }, nil +} + func getTelegramRepositoryPayload(p *api.RepositoryPayload) (*TelegramPayload, error) { var title string switch p.Action { @@ -192,6 +211,8 @@ func GetTelegramPayload(p api.Payloader, event models.HookEventType, meta string return getTelegramPushPayload(p.(*api.PushPayload)) case models.HookEventPullRequest: return getTelegramPullRequestPayload(p.(*api.PullRequestPayload)) + case models.HookEventPullRequestRejected, models.HookEventPullRequestApproved, models.HookEventPullRequestComment: + return getTelegramPullRequestApprovalPayload(p.(*api.PullRequestPayload), event) case models.HookEventRepository: return getTelegramRepositoryPayload(p.(*api.RepositoryPayload)) case models.HookEventRelease: From 14a96874442a13bb212affb13a585f0536d89c2a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 8 Jan 2020 22:14:00 +0100 Subject: [PATCH 6/9] times Add filters (#9373) (extend #9200) * add query param for GET functions (created Bevore & after) * add test * generalize func GetQueryBeforeSince Co-authored-by: Lunny Xiao --- integrations/api_issue_tracked_time_test.go | 12 ++ models/issue_tracked_time.go | 16 ++- routers/api/v1/api.go | 8 +- routers/api/v1/repo/issue_tracked_time.go | 134 +++++++++++++++++--- routers/api/v1/utils/utils.go | 33 ++++- templates/swagger/v1_json.tmpl | 63 ++++++++- 6 files changed, 234 insertions(+), 32 deletions(-) diff --git a/integrations/api_issue_tracked_time_test.go b/integrations/api_issue_tracked_time_test.go index ed6c036db6..97d401ff9d 100644 --- a/integrations/api_issue_tracked_time_test.go +++ b/integrations/api_issue_tracked_time_test.go @@ -44,6 +44,18 @@ func TestAPIGetTrackedTimes(t *testing.T) { assert.NoError(t, err) assert.Equal(t, user.Name, apiTimes[i].UserName) } + + // test filter + since := "2000-01-01T00%3A00%3A02%2B00%3A00" //946684802 + before := "2000-01-01T00%3A00%3A12%2B00%3A00" //946684812 + + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/%d/times?since=%s&before=%s&token=%s", user2.Name, issue2.Repo.Name, issue2.Index, since, before, token) + resp = session.MakeRequest(t, req, http.StatusOK) + var filterAPITimes api.TrackedTimeList + DecodeJSON(t, resp, &filterAPITimes) + assert.Len(t, filterAPITimes, 2) + assert.Equal(t, int64(3), filterAPITimes[0].ID) + assert.Equal(t, int64(6), filterAPITimes[1].ID) } func TestAPIDeleteTrackedTime(t *testing.T) { diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go index bcb163f3c5..b84adbc59a 100644 --- a/models/issue_tracked_time.go +++ b/models/issue_tracked_time.go @@ -100,10 +100,12 @@ func (tl TrackedTimeList) APIFormat() api.TrackedTimeList { // FindTrackedTimesOptions represent the filters for tracked times. If an ID is 0 it will be ignored. type FindTrackedTimesOptions struct { - IssueID int64 - UserID int64 - RepositoryID int64 - MilestoneID int64 + IssueID int64 + UserID int64 + RepositoryID int64 + MilestoneID int64 + CreatedAfterUnix int64 + CreatedBeforeUnix int64 } // ToCond will convert each condition into a xorm-Cond @@ -121,6 +123,12 @@ func (opts *FindTrackedTimesOptions) ToCond() builder.Cond { if opts.MilestoneID != 0 { cond = cond.And(builder.Eq{"issue.milestone_id": opts.MilestoneID}) } + if opts.CreatedAfterUnix != 0 { + cond = cond.And(builder.Gte{"tracked_time.created_unix": opts.CreatedAfterUnix}) + } + if opts.CreatedBeforeUnix != 0 { + cond = cond.And(builder.Lte{"tracked_time.created_unix": opts.CreatedBeforeUnix}) + } return cond } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 3f766c7a74..9f18951893 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -654,7 +654,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/times", func() { m.Combo("").Get(repo.ListTrackedTimesByRepository) m.Combo("/:timetrackingusername").Get(repo.ListTrackedTimesByUser) - }, mustEnableIssues) + }, mustEnableIssues, reqToken()) m.Group("/issues", func() { m.Combo("").Get(repo.ListIssues). Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), repo.CreateIssue) @@ -688,12 +688,12 @@ func RegisterRoutes(m *macaron.Macaron) { m.Delete("/:id", reqToken(), repo.DeleteIssueLabel) }) m.Group("/times", func() { - m.Combo("", reqToken()). + m.Combo(""). Get(repo.ListTrackedTimes). Post(bind(api.AddTimeOption{}), repo.AddTime). Delete(repo.ResetIssueTime) - m.Delete("/:id", reqToken(), repo.DeleteTime) - }) + m.Delete("/:id", repo.DeleteTime) + }, reqToken()) m.Combo("/deadline").Post(reqToken(), bind(api.EditDeadlineOption{}), repo.UpdateIssueDeadline) m.Group("/stopwatch", func() { m.Post("/start", reqToken(), repo.StartIssueStopwatch) diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index 80830e2fe6..dd959192c9 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -5,12 +5,15 @@ package repo import ( + "fmt" "net/http" + "strings" "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/routers/api/v1/utils" ) // ListTrackedTimes list all the tracked times of an issue @@ -37,6 +40,16 @@ func ListTrackedTimes(ctx *context.APIContext) { // type: integer // format: int64 // required: true + // - name: since + // in: query + // description: Only show times updated after the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time + // - name: before + // in: query + // description: Only show times updated before the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time // responses: // "200": // "$ref": "#/responses/TrackedTimeList" @@ -62,6 +75,11 @@ func ListTrackedTimes(ctx *context.APIContext) { IssueID: issue.ID, } + if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil { + ctx.InternalServerError(err) + return + } + if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin { opts.UserID = ctx.User.ID } @@ -141,7 +159,7 @@ func AddTime(ctx *context.APIContext, form api.AddTimeOption) { //allow only RepoAdmin, Admin and User to add time user, err = models.GetUserByName(form.User) if err != nil { - ctx.Error(500, "GetUserByName", err) + ctx.Error(http.StatusInternalServerError, "GetUserByName", err) } } } @@ -195,33 +213,33 @@ func ResetIssueTime(ctx *context.APIContext) { // "400": // "$ref": "#/responses/error" // "403": - // "$ref": "#/responses/error" + // "$ref": "#/responses/forbidden" issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { ctx.NotFound(err) } else { - ctx.Error(500, "GetIssueByIndex", err) + ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } return } if !ctx.Repo.CanUseTimetracker(issue, ctx.User) { if !ctx.Repo.Repository.IsTimetrackerEnabled() { - ctx.JSON(400, struct{ Message string }{Message: "time tracking disabled"}) + ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"}) return } - ctx.Status(403) + ctx.Status(http.StatusForbidden) return } err = models.DeleteIssueUserTimes(issue, ctx.User) if err != nil { if models.IsErrNotExist(err) { - ctx.Error(404, "DeleteIssueUserTimes", err) + ctx.Error(http.StatusNotFound, "DeleteIssueUserTimes", err) } else { - ctx.Error(500, "DeleteIssueUserTimes", err) + ctx.Error(http.StatusInternalServerError, "DeleteIssueUserTimes", err) } return } @@ -266,52 +284,53 @@ func DeleteTime(ctx *context.APIContext) { // "400": // "$ref": "#/responses/error" // "403": - // "$ref": "#/responses/error" + // "$ref": "#/responses/forbidden" issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { ctx.NotFound(err) } else { - ctx.Error(500, "GetIssueByIndex", err) + ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } return } if !ctx.Repo.CanUseTimetracker(issue, ctx.User) { if !ctx.Repo.Repository.IsTimetrackerEnabled() { - ctx.JSON(400, struct{ Message string }{Message: "time tracking disabled"}) + ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"}) return } - ctx.Status(403) + ctx.Status(http.StatusForbidden) return } time, err := models.GetTrackedTimeByID(ctx.ParamsInt64(":id")) if err != nil { - ctx.Error(500, "GetTrackedTimeByID", err) + ctx.Error(http.StatusInternalServerError, "GetTrackedTimeByID", err) return } if !ctx.User.IsAdmin && time.UserID != ctx.User.ID { //Only Admin and User itself can delete their time - ctx.Status(403) + ctx.Status(http.StatusForbidden) return } err = models.DeleteTime(time) if err != nil { - ctx.Error(500, "DeleteTime", err) + ctx.Error(http.StatusInternalServerError, "DeleteTime", err) return } - ctx.Status(204) + ctx.Status(http.StatusNoContent) } // ListTrackedTimesByUser lists all tracked times of the user func ListTrackedTimesByUser(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/times/{user} user userTrackedTimes + // swagger:operation GET /repos/{owner}/{repo}/times/{user} repository userTrackedTimes // --- // summary: List a user's tracked times in a repo + // deprecated: true // produces: // - application/json // parameters: @@ -335,6 +354,8 @@ func ListTrackedTimesByUser(ctx *context.APIContext) { // "$ref": "#/responses/TrackedTimeList" // "400": // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" if !ctx.Repo.Repository.IsTimetrackerEnabled() { ctx.Error(http.StatusBadRequest, "", "time tracking disabled") @@ -353,9 +374,23 @@ func ListTrackedTimesByUser(ctx *context.APIContext) { ctx.NotFound() return } - trackedTimes, err := models.GetTrackedTimes(models.FindTrackedTimesOptions{ + + if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin && ctx.User.ID != user.ID { + ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights")) + return + } + + if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin && ctx.User.ID != user.ID { + ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights")) + return + } + + opts := models.FindTrackedTimesOptions{ UserID: user.ID, - RepositoryID: ctx.Repo.Repository.ID}) + RepositoryID: ctx.Repo.Repository.ID, + } + + trackedTimes, err := models.GetTrackedTimes(opts) if err != nil { ctx.Error(http.StatusInternalServerError, "GetTrackedTimes", err) return @@ -385,11 +420,27 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: user + // in: query + // description: optional filter by user + // type: string + // - name: since + // in: query + // description: Only show times updated after the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time + // - name: before + // in: query + // description: Only show times updated before the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time // responses: // "200": // "$ref": "#/responses/TrackedTimeList" // "400": // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" if !ctx.Repo.Repository.IsTimetrackerEnabled() { ctx.Error(http.StatusBadRequest, "", "time tracking disabled") @@ -400,8 +451,30 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) { RepositoryID: ctx.Repo.Repository.ID, } + // Filters + qUser := strings.Trim(ctx.Query("user"), " ") + if qUser != "" { + user, err := models.GetUserByName(qUser) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetUserByName", err) + return + } + opts.UserID = user.ID + } + + var err error + if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil { + ctx.InternalServerError(err) + return + } + if !ctx.IsUserRepoAdmin() && !ctx.User.IsAdmin { - opts.UserID = ctx.User.ID + if opts.UserID == 0 { + opts.UserID = ctx.User.ID + } else { + ctx.Error(http.StatusForbidden, "", fmt.Errorf("query user not allowed not enouth rights")) + return + } } trackedTimes, err := models.GetTrackedTimes(opts) @@ -423,18 +496,39 @@ func ListMyTrackedTimes(ctx *context.APIContext) { // summary: List the current user's tracked times // produces: // - application/json + // parameters: + // - name: since + // in: query + // description: Only show times updated after the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time + // - name: before + // in: query + // description: Only show times updated before the given time. This is a timestamp in RFC 3339 format + // type: string + // format: date-time // responses: // "200": // "$ref": "#/responses/TrackedTimeList" - trackedTimes, err := models.GetTrackedTimes(models.FindTrackedTimesOptions{UserID: ctx.User.ID}) + opts := models.FindTrackedTimesOptions{UserID: ctx.User.ID} + + var err error + if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil { + ctx.InternalServerError(err) + return + } + + trackedTimes, err := models.GetTrackedTimes(opts) if err != nil { ctx.Error(http.StatusInternalServerError, "GetTrackedTimesByUser", err) return } + if err = trackedTimes.LoadAttributes(); err != nil { ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return } + ctx.JSON(http.StatusOK, trackedTimes.APIFormat()) } diff --git a/routers/api/v1/utils/utils.go b/routers/api/v1/utils/utils.go index f7c2b224d5..35f4873964 100644 --- a/routers/api/v1/utils/utils.go +++ b/routers/api/v1/utils/utils.go @@ -4,7 +4,12 @@ package utils -import "code.gitea.io/gitea/modules/context" +import ( + "strings" + "time" + + "code.gitea.io/gitea/modules/context" +) // UserID user ID of authenticated user, or 0 if not authenticated func UserID(ctx *context.APIContext) int64 { @@ -13,3 +18,29 @@ func UserID(ctx *context.APIContext) int64 { } return ctx.User.ID } + +// GetQueryBeforeSince return parsed time (unix format) from URL query's before and since +func GetQueryBeforeSince(ctx *context.APIContext) (before, since int64, err error) { + qCreatedBefore := strings.Trim(ctx.Query("before"), " ") + if qCreatedBefore != "" { + createdBefore, err := time.Parse(time.RFC3339, qCreatedBefore) + if err != nil { + return 0, 0, err + } + if !createdBefore.IsZero() { + before = createdBefore.Unix() + } + } + + qCreatedAfter := strings.Trim(ctx.Query("since"), " ") + if qCreatedAfter != "" { + createdAfter, err := time.Parse(time.RFC3339, qCreatedAfter) + if err != nil { + return 0, 0, err + } + if !createdAfter.IsZero() { + since = createdAfter.Unix() + } + } + return before, since, nil +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index f37c900cca..d3e2ac6113 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4433,6 +4433,20 @@ "name": "index", "in": "path", "required": true + }, + { + "type": "string", + "format": "date-time", + "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format", + "name": "since", + "in": "query" + }, + { + "type": "string", + "format": "date-time", + "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format", + "name": "before", + "in": "query" } ], "responses": { @@ -4543,7 +4557,7 @@ "$ref": "#/responses/error" }, "403": { - "$ref": "#/responses/error" + "$ref": "#/responses/forbidden" } } } @@ -4601,7 +4615,7 @@ "$ref": "#/responses/error" }, "403": { - "$ref": "#/responses/error" + "$ref": "#/responses/forbidden" } } } @@ -6419,6 +6433,26 @@ "name": "repo", "in": "path", "required": true + }, + { + "type": "string", + "description": "optional filter by user", + "name": "user", + "in": "query" + }, + { + "type": "string", + "format": "date-time", + "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format", + "name": "since", + "in": "query" + }, + { + "type": "string", + "format": "date-time", + "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format", + "name": "before", + "in": "query" } ], "responses": { @@ -6427,6 +6461,9 @@ }, "400": { "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" } } } @@ -6437,10 +6474,11 @@ "application/json" ], "tags": [ - "user" + "repository" ], "summary": "List a user's tracked times in a repo", "operationId": "userTrackedTimes", + "deprecated": true, "parameters": [ { "type": "string", @@ -6470,6 +6508,9 @@ }, "400": { "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" } } } @@ -7685,6 +7726,22 @@ ], "summary": "List the current user's tracked times", "operationId": "userCurrentTrackedTimes", + "parameters": [ + { + "type": "string", + "format": "date-time", + "description": "Only show times updated after the given time. This is a timestamp in RFC 3339 format", + "name": "since", + "in": "query" + }, + { + "type": "string", + "format": "date-time", + "description": "Only show times updated before the given time. This is a timestamp in RFC 3339 format", + "name": "before", + "in": "query" + } + ], "responses": { "200": { "$ref": "#/responses/TrackedTimeList" From 5b2d9333f1d06a15f11906b39c4867cc5d1c9448 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Wed, 8 Jan 2020 17:10:34 -0600 Subject: [PATCH 7/9] Add HTML URL to API Issues (#9654) * Add HTML URL to API Issues Signed-off-by: jolheiser * Swagger Signed-off-by: jolheiser Co-authored-by: Lauris BH --- models/issue.go | 1 + modules/structs/issue.go | 1 + modules/webhook/dingtalk.go | 2 +- modules/webhook/discord.go | 2 +- modules/webhook/msteams.go | 2 +- modules/webhook/slack.go | 2 +- templates/swagger/v1_json.tmpl | 4 ++++ 7 files changed, 10 insertions(+), 4 deletions(-) diff --git a/models/issue.go b/models/issue.go index 485be4baef..3986aeee15 100644 --- a/models/issue.go +++ b/models/issue.go @@ -381,6 +381,7 @@ func (issue *Issue) apiFormat(e Engine) *api.Issue { apiIssue := &api.Issue{ ID: issue.ID, URL: issue.APIURL(), + HTMLURL: issue.HTMLURL(), Index: issue.Index, Poster: issue.Poster.APIFormat(), Title: issue.Title, diff --git a/modules/structs/issue.go b/modules/structs/issue.go index cc640edab0..49a7dc6d6f 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -38,6 +38,7 @@ type RepositoryMeta struct { type Issue struct { ID int64 `json:"id"` URL string `json:"url"` + HTMLURL string `json:"html_url"` Index int64 `json:"number"` Poster *User `json:"user"` OriginalAuthor string `json:"original_author"` diff --git a/modules/webhook/dingtalk.go b/modules/webhook/dingtalk.go index 0d569f4120..fc99202a70 100644 --- a/modules/webhook/dingtalk.go +++ b/modules/webhook/dingtalk.go @@ -142,7 +142,7 @@ func getDingtalkIssuesPayload(p *api.IssuePayload) (*DingtalkPayload, error) { Title: issueTitle, HideAvatar: "0", SingleTitle: "view issue", - SingleURL: p.Issue.URL, + SingleURL: p.Issue.HTMLURL, }, }, nil } diff --git a/modules/webhook/discord.go b/modules/webhook/discord.go index c1e8421228..732821c183 100644 --- a/modules/webhook/discord.go +++ b/modules/webhook/discord.go @@ -236,7 +236,7 @@ func getDiscordIssuesPayload(p *api.IssuePayload, meta *DiscordMeta) (*DiscordPa { Title: text, Description: attachmentText, - URL: p.Issue.URL, + URL: p.Issue.HTMLURL, Color: color, Author: DiscordEmbedAuthor{ Name: p.Sender.UserName, diff --git a/modules/webhook/msteams.go b/modules/webhook/msteams.go index b47725209d..b9ceb5ee0b 100644 --- a/modules/webhook/msteams.go +++ b/modules/webhook/msteams.go @@ -299,7 +299,7 @@ func getMSTeamsIssuesPayload(p *api.IssuePayload) (*MSTeamsPayload, error) { Targets: []MSTeamsActionTarget{ { Os: "default", - URI: p.Issue.URL, + URI: p.Issue.HTMLURL, }, }, }, diff --git a/modules/webhook/slack.go b/modules/webhook/slack.go index 11ad4c1b8b..74361509d8 100644 --- a/modules/webhook/slack.go +++ b/modules/webhook/slack.go @@ -158,7 +158,7 @@ func getSlackIssuesPayload(p *api.IssuePayload, slack *SlackMeta) (*SlackPayload pl.Attachments = []SlackAttachment{{ Color: fmt.Sprintf("%x", color), Title: issueTitle, - TitleLink: p.Issue.URL, + TitleLink: p.Issue.HTMLURL, Text: attachmentText, }} } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d3e2ac6113..4e37f65d19 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -10305,6 +10305,10 @@ "format": "date-time", "x-go-name": "Deadline" }, + "html_url": { + "type": "string", + "x-go-name": "HTMLURL" + }, "id": { "type": "integer", "format": "int64", From 25531c71a78b98af91f25d5e6eaa362e5fc54a86 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Thu, 9 Jan 2020 02:47:45 +0100 Subject: [PATCH 8/9] Mark PR reviews as stale at push and allow to dismiss stale approvals (#9532) Fix #5997. If a push causes the patch/diff of a PR towards target branch to change, all existing reviews for the PR will be set and shown as stale. New branch protection option to dismiss stale approvals are added. To show that a review is not based on the latest PR changes, an hourglass is shown --- models/branches.go | 41 ++++---- models/migrations/migrations.go | 2 + models/migrations/v118.go | 26 +++++ models/review.go | 30 +++++- modules/auth/repo_form.go | 19 ++-- modules/git/repo_compare.go | 6 ++ modules/repofiles/update.go | 4 +- options/locale/locale_en-US.ini | 2 + routers/repo/pull.go | 2 +- routers/repo/pull_review.go | 4 +- routers/repo/setting_protected_branch.go | 1 + services/pull/merge.go | 2 +- services/pull/pull.go | 96 ++++++++++++++++++- services/pull/review.go | 37 +++++-- templates/repo/diff/comment_form.tmpl | 1 + templates/repo/diff/new_review.tmpl | 1 + templates/repo/issue/view_content/pull.tmpl | 5 + templates/repo/settings/protected_branch.tmpl | 8 ++ 18 files changed, 244 insertions(+), 43 deletions(-) create mode 100644 models/migrations/v118.go diff --git a/models/branches.go b/models/branches.go index 385817e4f9..1932e06db3 100644 --- a/models/branches.go +++ b/models/branches.go @@ -32,21 +32,23 @@ type ProtectedBranch struct { BranchName string `xorm:"UNIQUE(s)"` CanPush bool `xorm:"NOT NULL DEFAULT false"` EnableWhitelist bool - WhitelistUserIDs []int64 `xorm:"JSON TEXT"` - WhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` - WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` - MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` - MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` - StatusCheckContexts []string `xorm:"JSON TEXT"` - EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"` - ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` - ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` - RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` - BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` - CreatedUnix timeutil.TimeStamp `xorm:"created"` - UpdatedUnix timeutil.TimeStamp `xorm:"updated"` + WhitelistUserIDs []int64 `xorm:"JSON TEXT"` + WhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"` + WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` + MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` + MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"` + StatusCheckContexts []string `xorm:"JSON TEXT"` + EnableApprovalsWhitelist bool `xorm:"NOT NULL DEFAULT false"` + ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"` + ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` + BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` + DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` + + CreatedUnix timeutil.TimeStamp `xorm:"created"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated"` } // IsProtected returns if the branch is protected @@ -155,10 +157,13 @@ func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool { // GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist. func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 { - approvals, err := x.Where("issue_id = ?", pr.IssueID). + sess := x.Where("issue_id = ?", pr.IssueID). And("type = ?", ReviewTypeApprove). - And("official = ?", true). - Count(new(Review)) + And("official = ?", true) + if protectBranch.DismissStaleApprovals { + sess = sess.And("stale = ?", false) + } + approvals, err := sess.Count(new(Review)) if err != nil { log.Error("GetGrantedApprovalsCount: %v", err) return 0 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 73c9bc1138..f26566b045 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -290,6 +290,8 @@ var migrations = []Migration{ NewMigration("Extend TrackedTimes", extendTrackedTimes), // v117 -> v118 NewMigration("Add block on rejected reviews branch protection", addBlockOnRejectedReviews), + // v118 -> v119 + NewMigration("Add commit id and stale to reviews", addReviewCommitAndStale), } // Migrate database to current version diff --git a/models/migrations/v118.go b/models/migrations/v118.go new file mode 100644 index 0000000000..c79cbb8ae3 --- /dev/null +++ b/models/migrations/v118.go @@ -0,0 +1,26 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" +) + +func addReviewCommitAndStale(x *xorm.Engine) error { + type Review struct { + CommitID string `xorm:"VARCHAR(40)"` + Stale bool `xorm:"NOT NULL DEFAULT false"` + } + + type ProtectedBranch struct { + DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` + } + + // Old reviews will have commit ID set to "" and not stale + if err := x.Sync2(new(Review)); err != nil { + return err + } + return x.Sync2(new(ProtectedBranch)) +} diff --git a/models/review.go b/models/review.go index bc7dfbcd14..2838cfa316 100644 --- a/models/review.go +++ b/models/review.go @@ -53,7 +53,9 @@ type Review struct { IssueID int64 `xorm:"index"` Content string `xorm:"TEXT"` // Official is a review made by an assigned approver (counts towards approval) - Official bool `xorm:"NOT NULL DEFAULT false"` + Official bool `xorm:"NOT NULL DEFAULT false"` + CommitID string `xorm:"VARCHAR(40)"` + Stale bool `xorm:"NOT NULL DEFAULT false"` CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` @@ -169,6 +171,8 @@ type CreateReviewOptions struct { Issue *Issue Reviewer *User Official bool + CommitID string + Stale bool } // IsOfficialReviewer check if reviewer can make official reviews in issue (counts towards required approvals) @@ -200,6 +204,8 @@ func createReview(e Engine, opts CreateReviewOptions) (*Review, error) { ReviewerID: opts.Reviewer.ID, Content: opts.Content, Official: opts.Official, + CommitID: opts.CommitID, + Stale: opts.Stale, } if _, err := e.Insert(review); err != nil { return nil, err @@ -258,7 +264,7 @@ func IsContentEmptyErr(err error) bool { } // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist -func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content string) (*Review, *Comment, error) { +func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content, commitID string, stale bool) (*Review, *Comment, error) { sess := x.NewSession() defer sess.Close() if err := sess.Begin(); err != nil { @@ -295,6 +301,8 @@ func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content strin Reviewer: doer, Content: content, Official: official, + CommitID: commitID, + Stale: stale, }) if err != nil { return nil, nil, err @@ -322,8 +330,10 @@ func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content strin review.Issue = issue review.Content = content review.Type = reviewType + review.CommitID = commitID + review.Stale = stale - if _, err := sess.ID(review.ID).Cols("content, type, official").Update(review); err != nil { + if _, err := sess.ID(review.ID).Cols("content, type, official, commit_id, stale").Update(review); err != nil { return nil, nil, err } } @@ -374,3 +384,17 @@ func GetReviewersByIssueID(issueID int64) (reviews []*Review, err error) { return reviews, nil } + +// MarkReviewsAsStale marks existing reviews as stale +func MarkReviewsAsStale(issueID int64) (err error) { + _, err = x.Exec("UPDATE `review` SET stale=? WHERE issue_id=?", true, issueID) + + return +} + +// MarkReviewsAsNotStale marks existing reviews as not stale for a giving commit SHA +func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) { + _, err = x.Exec("UPDATE `review` SET stale=? WHERE issue_id=? AND commit_id=?", false, issueID, commitID) + + return +} diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index c87549af92..28615ebbc4 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -172,6 +172,7 @@ type ProtectBranchForm struct { ApprovalsWhitelistUsers string ApprovalsWhitelistTeams string BlockOnRejectedReviews bool + DismissStaleApprovals bool } // Validate validates the fields @@ -456,12 +457,13 @@ func (f *MergePullRequestForm) Validate(ctx *macaron.Context, errs binding.Error // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { - Content string `binding:"Required"` - Side string `binding:"Required;In(previous,proposed)"` - Line int64 - TreePath string `form:"path" binding:"Required"` - IsReview bool `form:"is_review"` - Reply int64 `form:"reply"` + Content string `binding:"Required"` + Side string `binding:"Required;In(previous,proposed)"` + Line int64 + TreePath string `form:"path" binding:"Required"` + IsReview bool `form:"is_review"` + Reply int64 `form:"reply"` + LatestCommitID string } // Validate validates the fields @@ -471,8 +473,9 @@ func (f *CodeCommentForm) Validate(ctx *macaron.Context, errs binding.Errors) bi // SubmitReviewForm for submitting a finished code review type SubmitReviewForm struct { - Content string - Type string `binding:"Required;In(approve,comment,reject)"` + Content string + Type string `binding:"Required;In(approve,comment,reject)"` + CommitID string } // Validate validates the fields diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 53b8af4bb4..683ac7a7ee 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -112,3 +112,9 @@ func (repo *Repository) GetPatch(base, head string, w io.Writer) error { return NewCommand("format-patch", "--binary", "--stdout", base+"..."+head). RunInDirPipeline(repo.Path, w, nil) } + +// GetDiffFromMergeBase generates and return patch data from merge base to head +func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) error { + return NewCommand("diff", "-p", "--binary", base+"..."+head). + RunInDirPipeline(repo.Path, w, nil) +} diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index 19c3d5b51c..7ad4a4d388 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -477,7 +477,7 @@ func PushUpdate(repo *models.Repository, branch string, opts PushUpdateOptions) log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name) - go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true) + go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, opts.OldCommitID, opts.NewCommitID) if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil { log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err) @@ -528,7 +528,7 @@ func PushUpdates(repo *models.Repository, optsList []*PushUpdateOptions) error { log.Trace("TriggerTask '%s/%s' by %s", repo.Name, opts.Branch, pusher.Name) - go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true) + go pull_service.AddTestPullRequestTask(pusher, repo.ID, opts.Branch, true, opts.OldCommitID, opts.NewCommitID) if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil { log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f6ff12250a..a00df07d93 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1413,6 +1413,8 @@ settings.protect_approvals_whitelist_enabled = Restrict approvals to whitelisted settings.protect_approvals_whitelist_enabled_desc = Only reviews from whitelisted users or teams will count to the required approvals. Without approval whitelist, reviews from anyone with write access count to the required approvals. settings.protect_approvals_whitelist_users = Whitelisted reviewers: settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews: +settings.dismiss_stale_approvals = Dismiss stale approvals +settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed. settings.add_protected_branch = Enable protection settings.delete_protected_branch = Disable protection settings.update_protect_branch_success = Branch protection for branch '%s' has been updated. diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 1a5c4a036f..93e98e0c33 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -841,7 +841,7 @@ func TriggerTask(ctx *context.Context) { log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name) - go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true) + go pull_service.AddTestPullRequestTask(pusher, repo.ID, branch, true, "", "") ctx.Status(202) } diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index b596d2578b..558473eb08 100644 --- a/routers/repo/pull_review.go +++ b/routers/repo/pull_review.go @@ -37,12 +37,14 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) { comment, err := pull_service.CreateCodeComment( ctx.User, + ctx.Repo.GitRepo, issue, signedLine, form.Content, form.TreePath, form.IsReview, form.Reply, + form.LatestCommitID, ) if err != nil { ctx.ServerError("CreateCodeComment", err) @@ -95,7 +97,7 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) { } } - _, comm, err := pull_service.SubmitReview(ctx.User, issue, reviewType, form.Content) + _, comm, err := pull_service.SubmitReview(ctx.User, ctx.Repo.GitRepo, issue, reviewType, form.Content, form.CommitID) if err != nil { if models.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index 8872c7471f..da28ac50be 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -245,6 +245,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) } } protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews + protectBranch.DismissStaleApprovals = f.DismissStaleApprovals err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ UserIDs: whitelistUsers, diff --git a/services/pull/merge.go b/services/pull/merge.go index 8aa38bf11e..b38c2e72f2 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -64,7 +64,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor } defer func() { - go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false) + go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "") }() // Clone base repo. diff --git a/services/pull/pull.go b/services/pull/pull.go index 6be9c2da17..b459d81cf7 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -5,10 +5,14 @@ package pull import ( + "bufio" + "bytes" "context" "fmt" "os" "path" + "strings" + "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" @@ -16,6 +20,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" issue_service "code.gitea.io/gitea/services/issue" + + "github.com/unknwon/com" ) // NewPullRequest creates new pull request with labels for repository. @@ -168,7 +174,7 @@ func addHeadRepoTasks(prs []*models.PullRequest) { // AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch, // and generate new patch for testing as needed. -func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSync bool) { +func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSync bool, oldCommitID, newCommitID string) { log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", repoID, branch) graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) { // There is no sensible way to shut this down ":-(" @@ -191,6 +197,22 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy } if err == nil { for _, pr := range prs { + if newCommitID != "" && newCommitID != git.EmptySHA { + changed, err := checkIfPRContentChanged(pr, oldCommitID, newCommitID) + if err != nil { + log.Error("checkIfPRContentChanged: %v", err) + } + if changed { + // Mark old reviews as stale if diff to mergebase has changed + if err := models.MarkReviewsAsStale(pr.IssueID); err != nil { + log.Error("MarkReviewsAsStale: %v", err) + } + } + if err := models.MarkReviewsAsNotStale(pr.IssueID, newCommitID); err != nil { + log.Error("MarkReviewsAsNotStale: %v", err) + } + } + pr.Issue.PullRequest = pr notification.NotifyPullRequestSynchronized(doer, pr) } @@ -211,6 +233,78 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy }) } +// checkIfPRContentChanged checks if diff to target branch has changed by push +// A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged +func checkIfPRContentChanged(pr *models.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) { + + if err = pr.GetHeadRepo(); err != nil { + return false, fmt.Errorf("GetHeadRepo: %v", err) + } else if pr.HeadRepo == nil { + // corrupt data assumed changed + return true, nil + } + + if err = pr.GetBaseRepo(); err != nil { + return false, fmt.Errorf("GetBaseRepo: %v", err) + } + + headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + return false, fmt.Errorf("OpenRepository: %v", err) + } + defer headGitRepo.Close() + + // Add a temporary remote. + tmpRemote := "checkIfPRContentChanged-" + com.ToStr(time.Now().UnixNano()) + if err = headGitRepo.AddRemote(tmpRemote, models.RepoPath(pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name), true); err != nil { + return false, fmt.Errorf("AddRemote: %s/%s-%s: %v", pr.HeadRepo.OwnerName, pr.HeadRepo.Name, tmpRemote, err) + } + defer func() { + if err := headGitRepo.RemoveRemote(tmpRemote); err != nil { + log.Error("checkIfPRContentChanged: RemoveRemote: %s/%s-%s: %v", pr.HeadRepo.OwnerName, pr.HeadRepo.Name, tmpRemote, err) + } + }() + // To synchronize repo and get a base ref + _, base, err := headGitRepo.GetMergeBase(tmpRemote, pr.BaseBranch, pr.HeadBranch) + if err != nil { + return false, fmt.Errorf("GetMergeBase: %v", err) + } + + diffBefore := &bytes.Buffer{} + diffAfter := &bytes.Buffer{} + if err := headGitRepo.GetDiffFromMergeBase(base, oldCommitID, diffBefore); err != nil { + // If old commit not found, assume changed. + log.Debug("GetDiffFromMergeBase: %v", err) + return true, nil + } + if err := headGitRepo.GetDiffFromMergeBase(base, newCommitID, diffAfter); err != nil { + // New commit should be found + return false, fmt.Errorf("GetDiffFromMergeBase: %v", err) + } + + diffBeforeLines := bufio.NewScanner(diffBefore) + diffAfterLines := bufio.NewScanner(diffAfter) + + for diffBeforeLines.Scan() && diffAfterLines.Scan() { + if strings.HasPrefix(diffBeforeLines.Text(), "index") && strings.HasPrefix(diffAfterLines.Text(), "index") { + // file hashes can change without the diff changing + continue + } else if strings.HasPrefix(diffBeforeLines.Text(), "@@") && strings.HasPrefix(diffAfterLines.Text(), "@@") { + // the location of the difference may change + continue + } else if !bytes.Equal(diffBeforeLines.Bytes(), diffAfterLines.Bytes()) { + return true, nil + } + } + + if diffBeforeLines.Scan() || diffAfterLines.Scan() { + // Diffs not of equal length + return true, nil + } + + return false, nil +} + // PushToBaseRepo pushes commits from branches of head repository to // corresponding branches of base repository. // FIXME: Only push branches that are actually updates? diff --git a/services/pull/review.go b/services/pull/review.go index 5b453e87f4..2bc8240230 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -18,7 +18,7 @@ import ( ) // CreateCodeComment creates a comment on the code line -func CreateCodeComment(doer *models.User, issue *models.Issue, line int64, content string, treePath string, isReview bool, replyReviewID int64) (*models.Comment, error) { +func CreateCodeComment(doer *models.User, gitRepo *git.Repository, issue *models.Issue, line int64, content string, treePath string, isReview bool, replyReviewID int64, latestCommitID string) (*models.Comment, error) { var ( existsReview bool @@ -73,6 +73,7 @@ func CreateCodeComment(doer *models.User, issue *models.Issue, line int64, conte Reviewer: doer, Issue: issue, Official: false, + CommitID: latestCommitID, }) if err != nil { return nil, err @@ -94,7 +95,7 @@ func CreateCodeComment(doer *models.User, issue *models.Issue, line int64, conte if !isReview && !existsReview { // Submit the review we've just created so the comment shows up in the issue view - if _, _, err = SubmitReview(doer, issue, models.ReviewTypeComment, ""); err != nil { + if _, _, err = SubmitReview(doer, gitRepo, issue, models.ReviewTypeComment, "", latestCommitID); err != nil { return nil, err } } @@ -159,16 +160,36 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models } // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist -func SubmitReview(doer *models.User, issue *models.Issue, reviewType models.ReviewType, content string) (*models.Review, *models.Comment, error) { - review, comm, err := models.SubmitReview(doer, issue, reviewType, content) - if err != nil { - return nil, nil, err - } - +func SubmitReview(doer *models.User, gitRepo *git.Repository, issue *models.Issue, reviewType models.ReviewType, content, commitID string) (*models.Review, *models.Comment, error) { pr, err := issue.GetPullRequest() if err != nil { return nil, nil, err } + + var stale bool + if reviewType != models.ReviewTypeApprove && reviewType != models.ReviewTypeReject { + stale = false + } else { + headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + return nil, nil, err + } + + if headCommitID == commitID { + stale = false + } else { + stale, err = checkIfPRContentChanged(pr, commitID, headCommitID) + if err != nil { + return nil, nil, err + } + } + } + + review, comm, err := models.SubmitReview(doer, issue, reviewType, content, commitID, stale) + if err != nil { + return nil, nil, err + } + notification.NotifyPullRequestReview(pr, review, comm) return review, comm, nil diff --git a/templates/repo/diff/comment_form.tmpl b/templates/repo/diff/comment_form.tmpl index 573e7d4638..822b1e54f6 100644 --- a/templates/repo/diff/comment_form.tmpl +++ b/templates/repo/diff/comment_form.tmpl @@ -4,6 +4,7 @@ {{end}}
{{$.root.CsrfTokenHtml}} + diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index 68d8f893f2..4981c9ef48 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -7,6 +7,7 @@
{{.CsrfTokenHtml}} +
{{$.i18n.Tr "repo.diff.review.header"}} diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 2dc76dcf2e..b1e6feeba0 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -13,6 +13,11 @@ {{else}}grey{{end}}"> + {{if .Stale}} + + + + {{end}} diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index bf74a12330..c6701ce8a9 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -211,6 +211,14 @@

{{.i18n.Tr "repo.settings.block_rejected_reviews_desc"}}

+
+
+ + +

{{.i18n.Tr "repo.settings.dismiss_stale_approvals_desc"}}

+
+
+
From ee9ce0cfa9c003b359d2d3fba9346fd0929e88f4 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Thu, 9 Jan 2020 00:22:37 -0600 Subject: [PATCH 9/9] Fix nil reference in repo generation (#9660) * Fix nil reference Signed-off-by: jolheiser * Tighten Signed-off-by: jolheiser --- models/repo_generate.go | 66 +++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/models/repo_generate.go b/models/repo_generate.go index 556a5fc2f7..1b0466eaa7 100644 --- a/models/repo_generate.go +++ b/models/repo_generate.go @@ -124,41 +124,43 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository, return fmt.Errorf("checkGiteaTemplate: %v", err) } - if err := os.Remove(gt.Path); err != nil { - return fmt.Errorf("remove .giteatemplate: %v", err) - } + if gt != nil { + if err := os.Remove(gt.Path); err != nil { + return fmt.Errorf("remove .giteatemplate: %v", err) + } - // Avoid walking tree if there are no globs - if len(gt.Globs()) > 0 { - tmpDirSlash := strings.TrimSuffix(filepath.ToSlash(tmpDir), "/") + "/" - if err := filepath.Walk(tmpDirSlash, func(path string, info os.FileInfo, walkErr error) error { - if walkErr != nil { - return walkErr - } - - if info.IsDir() { - return nil - } - - base := strings.TrimPrefix(filepath.ToSlash(path), tmpDirSlash) - for _, g := range gt.Globs() { - if g.Match(base) { - content, err := ioutil.ReadFile(path) - if err != nil { - return err - } - - if err := ioutil.WriteFile(path, - []byte(generateExpansion(string(content), templateRepo, generateRepo)), - 0644); err != nil { - return err - } - break + // Avoid walking tree if there are no globs + if len(gt.Globs()) > 0 { + tmpDirSlash := strings.TrimSuffix(filepath.ToSlash(tmpDir), "/") + "/" + if err := filepath.Walk(tmpDirSlash, func(path string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr } + + if info.IsDir() { + return nil + } + + base := strings.TrimPrefix(filepath.ToSlash(path), tmpDirSlash) + for _, g := range gt.Globs() { + if g.Match(base) { + content, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + if err := ioutil.WriteFile(path, + []byte(generateExpansion(string(content), templateRepo, generateRepo)), + 0644); err != nil { + return err + } + break + } + } + return nil + }); err != nil { + return err } - return nil - }); err != nil { - return err } }