fix(ui): display user-friendly message for range error (#7420)

- Instead of displaying 'RangeError: Range' display 'x must be a number between $MIN and $MAX' when the validation fails for a range error check.
- Resolves forgejo/forgejo#3510
- Added integration testing.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7420
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-04-02 15:48:09 +00:00 committed by Gusted
parent 15a2338ff2
commit 5706a2452e
4 changed files with 88 additions and 2 deletions

View file

@ -79,6 +79,11 @@ func GetInclude(field reflect.StructField) string {
return getRuleBody(field, "Include(")
}
func GetRange(field reflect.StructField) (string, string) {
min, max, _ := strings.Cut(getRuleBody(field, "Range("), ",")
return min, max
}
// Validate populates the data with validation error (if any).
func Validate(errs binding.Errors, data map[string]any, f any, l translation.Locale) binding.Errors {
if errs.Len() == 0 {
@ -131,6 +136,9 @@ func Validate(errs binding.Errors, data map[string]any, f any, l translation.Loc
data["ErrorMsg"] = trName + l.TrString("form.url_error", errs[0].Message)
case binding.ERR_INCLUDE:
data["ErrorMsg"] = trName + l.TrString("form.include_error", GetInclude(field))
case binding.ERR_RANGE:
min, max := GetRange(field)
data["ErrorMsg"] = trName + l.TrString("alert.range_error", l.PrettyNumber(min), l.PrettyNumber(max))
case validation.ErrGlobPattern:
data["ErrorMsg"] = trName + l.TrString("form.glob_pattern_error", errs[0].Message)
case validation.ErrRegexPattern:

View file

@ -19,6 +19,7 @@
"themes.names.forgejo-dark": "Forgejo dark",
"error.not_found.title": "Page not found",
"alert.asset_load_failed": "Failed to load asset files from {path}. Please make sure the asset files can be accessed.",
"alert.range_error": " must be a number between %[1]s and %[2]s.",
"settings.adopt": "Adopt",
"install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s",
"install.lfs_jwt_secret_failed": "Unable to generate a LFS JWT secret: %[1]s"

View file

@ -725,8 +725,8 @@ func (f *DeleteRepoFileForm) Validate(req *http.Request, errs binding.Errors) bi
// AddTimeManuallyForm form that adds spent time manually.
type AddTimeManuallyForm struct {
Hours int `binding:"Range(0,1000)"`
Minutes int `binding:"Range(0,1000)"`
Hours int `binding:"Range(0,1000)" locale:"repo.issues.add_time_hours"`
Minutes int `binding:"Range(0,1000)" locale:"repo.issues.add_time_minutes"`
}
// Validate validates the fields

View file

@ -0,0 +1,77 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
package integration
import (
"net/http"
"testing"
issues_model "forgejo.org/models/issues"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
forgejo_context "forgejo.org/services/context"
"forgejo.org/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestIssueAddTimeManually(t *testing.T) {
defer tests.PrepareTestEnv(t)()
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
session := loginUser(t, user2.Name)
issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
require.NoError(t, issue2.LoadRepo(t.Context()))
t.Run("No time", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
session.MakeRequest(t, NewRequestWithValues(t, "POST", issue2.Link()+"/times/add", map[string]string{
"_csrf": GetCSRF(t, session, issue2.Link()),
}), http.StatusSeeOther)
flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.Contains(t, flashCookie.Value, "error%3DNo%2Btime%2Bwas%2Bentered.")
})
t.Run("Invalid hours", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
session.MakeRequest(t, NewRequestWithValues(t, "POST", issue2.Link()+"/times/add", map[string]string{
"_csrf": GetCSRF(t, session, issue2.Link()),
"hours": "-1",
}), http.StatusSeeOther)
flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.Contains(t, flashCookie.Value, "error%3DHours%2Bmust%2Bbe%2Ba%2Bnumber%2Bbetween%2B0%2Band%2B1%252C000.")
})
t.Run("Invalid minutes", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
session.MakeRequest(t, NewRequestWithValues(t, "POST", issue2.Link()+"/times/add", map[string]string{
"_csrf": GetCSRF(t, session, issue2.Link()),
"minutes": "-1",
}), http.StatusSeeOther)
flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.Contains(t, flashCookie.Value, "error%3DMinutes%2Bmust%2Bbe%2Ba%2Bnumber%2Bbetween%2B0%2Band%2B1%252C000.")
})
t.Run("Normal", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
session.MakeRequest(t, NewRequestWithValues(t, "POST", issue2.Link()+"/times/add", map[string]string{
"_csrf": GetCSRF(t, session, issue2.Link()),
"hours": "3",
"minutes": "14",
}), http.StatusSeeOther)
unittest.AssertExistsIf(t, true, &issues_model.TrackedTime{IssueID: issue2.ID, Time: 11640, UserID: user2.ID})
})
}