Issue time estimate, meaningful time tracking (#23113)

Redesign the time tracker side bar, and add "time estimate" support (in "1d 2m" format)

Closes #23112

---------

Co-authored-by: stuzer05 <[email protected]>
Co-authored-by: Yarden Shoham <[email protected]>
Co-authored-by: silverwind <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 6 people

Diff for: models/issues/comment.go

@@ -114,6 +114,8 @@ const (
 
 	CommentTypePin   // 36 pin Issue
 	CommentTypeUnpin // 37 unpin Issue
+
+	CommentTypeChangeTimeEstimate // 38 Change time estimate
 )
 
 var commentStrings = []string{
@@ -155,6 +157,7 @@ var commentStrings = []string{
 	"pull_cancel_scheduled_merge",
 	"pin",
 	"unpin",
+	"change_time_estimate",
 }
 
 func (t CommentType) String() string {

Diff for: models/issues/issue.go

@@ -147,6 +147,9 @@ type Issue struct {
 
 	// For view issue page.
 	ShowRole RoleDescriptor `xorm:"-"`
+
+	// Time estimate
+	TimeEstimate int64 `xorm:"NOT NULL DEFAULT 0"`
 }
 
 var (
@@ -934,3 +937,28 @@ func insertIssue(ctx context.Context, issue *Issue) error {
 
 	return nil
 }
+
+// ChangeIssueTimeEstimate changes the plan time of this issue, as the given user.
+func ChangeIssueTimeEstimate(ctx context.Context, issue *Issue, doer *user_model.User, timeEstimate int64) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		if err := UpdateIssueCols(ctx, &Issue{ID: issue.ID, TimeEstimate: timeEstimate}, "time_estimate"); err != nil {
+			return fmt.Errorf("updateIssueCols: %w", err)
+		}
+
+		if err := issue.LoadRepo(ctx); err != nil {
+			return fmt.Errorf("loadRepo: %w", err)
+		}
+
+		opts := &CreateCommentOptions{
+			Type:    CommentTypeChangeTimeEstimate,
+			Doer:    doer,
+			Repo:    issue.Repo,
+			Issue:   issue,
+			Content: fmt.Sprintf("%d", timeEstimate),
+		}
+		if _, err := CreateComment(ctx, opts); err != nil {
+			return fmt.Errorf("createComment: %w", err)
+		}
+		return nil
+	})
+}

Diff for: models/migrations/migrations.go

@@ -368,6 +368,7 @@ func prepareMigrationTasks() []*migration {
 		newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard),
 		newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices),
 		newMigration(310, "Add Priority to ProtectedBranch", v1_23.AddPriorityToProtectedBranch),
+		newMigration(311, "Add TimeEstimate to Issue table", v1_23.AddTimeEstimateColumnToIssueTable),
 	}
 	return preparedMigrations
 }

Diff for: models/migrations/v1_23/v311.go

@@ -0,0 +1,16 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_23 //nolint
+
+import (
+	"xorm.io/xorm"
+)
+
+func AddTimeEstimateColumnToIssueTable(x *xorm.Engine) error {
+	type Issue struct {
+		TimeEstimate int64 `xorm:"NOT NULL DEFAULT 0"`
+	}
+
+	return x.Sync(new(Issue))
+}

Diff for: modules/templates/helper.go

@@ -70,6 +70,9 @@ func NewFuncMap() template.FuncMap {
 		"FileSize": base.FileSize,
 		"CountFmt": base.FormatNumberSI,
 		"Sec2Time": util.SecToTime,
+
+		"TimeEstimateString": timeEstimateString,
+
 		"LoadTimes": func(startTime time.Time) string {
 			return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms"
 		},
@@ -282,6 +285,14 @@ func userThemeName(user *user_model.User) string {
 	return setting.UI.DefaultTheme
 }
 
+func timeEstimateString(timeSec any) string {
+	v, _ := util.ToInt64(timeSec)
+	if v == 0 {
+		return ""
+	}
+	return util.TimeEstimateString(v)
+}
+
 func panicIfDevOrTesting() {
 	if !setting.IsProd || setting.IsInTesting {
 		panic("legacy template functions are for backward compatibility only, do not use them in new code")

Diff for: modules/util/time_str.go

@@ -0,0 +1,85 @@
+// Copyright 2024 Gitea. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package util
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+	"sync"
+)
+
+type timeStrGlobalVarsType struct {
+	units []struct {
+		name string
+		num  int64
+	}
+	re *regexp.Regexp
+}
+
+// When tracking working time, only hour/minute/second units are accurate and could be used.
+// For other units like "day", it depends on "how many working hours in a day": 6 or 7 or 8?
+// So at the moment, we only support hour/minute/second units.
+// In the future, it could be some configurable options to help users
+// to convert the working time to different units.
+
+var timeStrGlobalVars = sync.OnceValue[*timeStrGlobalVarsType](func() *timeStrGlobalVarsType {
+	v := &timeStrGlobalVarsType{}
+	v.re = regexp.MustCompile(`(?i)(\d+)\s*([hms])`)
+	v.units = []struct {
+		name string
+		num  int64
+	}{
+		{"h", 60 * 60},
+		{"m", 60},
+		{"s", 1},
+	}
+	return v
+})
+
+func TimeEstimateParse(timeStr string) (int64, error) {
+	if timeStr == "" {
+		return 0, nil
+	}
+	var total int64
+	matches := timeStrGlobalVars().re.FindAllStringSubmatchIndex(timeStr, -1)
+	if len(matches) == 0 {
+		return 0, fmt.Errorf("invalid time string: %s", timeStr)
+	}
+	if matches[0][0] != 0 || matches[len(matches)-1][1] != len(timeStr) {
+		return 0, fmt.Errorf("invalid time string: %s", timeStr)
+	}
+	for _, match := range matches {
+		amount, err := strconv.ParseInt(timeStr[match[2]:match[3]], 10, 64)
+		if err != nil {
+			return 0, fmt.Errorf("invalid time string: %v", err)
+		}
+		unit := timeStr[match[4]:match[5]]
+		found := false
+		for _, u := range timeStrGlobalVars().units {
+			if strings.ToLower(unit) == u.name {
+				total += amount * u.num
+				found = true
+				break
+			}
+		}
+		if !found {
+			return 0, fmt.Errorf("invalid time unit: %s", unit)
+		}
+	}
+	return total, nil
+}
+
+func TimeEstimateString(amount int64) string {
+	var timeParts []string
+	for _, u := range timeStrGlobalVars().units {
+		if amount >= u.num {
+			num := amount / u.num
+			amount %= u.num
+			timeParts = append(timeParts, fmt.Sprintf("%d%s", num, u.name))
+		}
+	}
+	return strings.Join(timeParts, " ")
+}

Diff for: modules/util/time_str_test.go

@@ -0,0 +1,55 @@
+// Copyright 2024 Gitea. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestTimeStr(t *testing.T) {
+	t.Run("Parse", func(t *testing.T) {
+		// Test TimeEstimateParse
+		tests := []struct {
+			input  string
+			output int64
+			err    bool
+		}{
+			{"1h", 3600, false},
+			{"1m", 60, false},
+			{"1s", 1, false},
+			{"1h 1m 1s", 3600 + 60 + 1, false},
+			{"1d1x", 0, true},
+		}
+		for _, test := range tests {
+			t.Run(test.input, func(t *testing.T) {
+				output, err := TimeEstimateParse(test.input)
+				if test.err {
+					assert.NotNil(t, err)
+				} else {
+					assert.Nil(t, err)
+				}
+				assert.Equal(t, test.output, output)
+			})
+		}
+	})
+	t.Run("String", func(t *testing.T) {
+		tests := []struct {
+			input  int64
+			output string
+		}{
+			{3600, "1h"},
+			{60, "1m"},
+			{1, "1s"},
+			{3600 + 1, "1h 1s"},
+		}
+		for _, test := range tests {
+			t.Run(test.output, func(t *testing.T) {
+				output := TimeEstimateString(test.input)
+				assert.Equal(t, test.output, output)
+			})
+		}
+	})
+}

Diff for: options/locale/locale_en-US.ini

@@ -1670,27 +1670,34 @@ issues.comment_on_locked = You cannot comment on a locked issue.
 issues.delete = Delete
 issues.delete.title = Delete this issue?
 issues.delete.text = Do you really want to delete this issue? (This will permanently remove all content. Consider closing it instead, if you intend to keep it archived)
+
 issues.tracker = Time Tracker
-issues.start_tracking_short = Start Timer
-issues.start_tracking = Start Time Tracking
-issues.start_tracking_history = `started working %s`
+issues.timetracker_timer_start = Start timer
+issues.timetracker_timer_stop = Stop timer
+issues.timetracker_timer_discard = Discard timer
+issues.timetracker_timer_manually_add = Add Time
+
+issues.time_estimate_placeholder = 1h 2m
+issues.time_estimate_set = Set estimated time
+issues.time_estimate_display = Estimate: %s
+issues.change_time_estimate_at = changed time estimate to <b>%s</b> %s
+issues.remove_time_estimate_at = removed time estimate %s
+issues.time_estimate_invalid = Time estimate format is invalid
+issues.start_tracking_history = started working %s
 issues.tracker_auto_close = Timer will be stopped automatically when this issue gets closed
 issues.tracking_already_started = `You have already started time tracking on <a href="%s">another issue</a>!`
-issues.stop_tracking = Stop Timer
-issues.stop_tracking_history = `stopped working %s`
-issues.cancel_tracking = Discard
+issues.stop_tracking_history = worked for <b>%s</b> %s
 issues.cancel_tracking_history = `canceled time tracking %s`
-issues.add_time = Manually Add Time
 issues.del_time = Delete this time log
-issues.add_time_short = Add Time
-issues.add_time_cancel = Cancel
-issues.add_time_history = `added spent time %s`
+issues.add_time_history = added spent time <b>%s</b> %s
 issues.del_time_history= `deleted spent time %s`
+issues.add_time_manually = Manually Add Time
 issues.add_time_hours = Hours
 issues.add_time_minutes = Minutes
 issues.add_time_sum_to_small = No time was entered.
 issues.time_spent_total = Total Time Spent
 issues.time_spent_from_all_authors = `Total Time Spent: %s`
+
 issues.due_date = Due Date
 issues.invalid_due_date_format = "Due date format must be 'yyyy-mm-dd'."
 issues.error_modifying_due_date = "Failed to modify the due date."

Diff for: routers/web/repo/issue_stopwatch.go

@@ -4,7 +4,6 @@
 package repo
 
 import (
-	"net/http"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
@@ -40,8 +39,7 @@ func IssueStopwatch(c *context.Context) {
 		c.Flash.Success(c.Tr("repo.issues.tracker_auto_close"))
 	}
 
-	url := issue.Link()
-	c.Redirect(url, http.StatusSeeOther)
+	c.JSONRedirect("")
 }
 
 // CancelStopwatch cancel the stopwatch
@@ -72,8 +70,7 @@ func CancelStopwatch(c *context.Context) {
 		})
 	}
 
-	url := issue.Link()
-	c.Redirect(url, http.StatusSeeOther)
+	c.JSONRedirect("")
 }
 
 // GetActiveStopwatch is the middleware that sets .ActiveStopwatch on context