Skip to content

Commit

Permalink
Unify repo settings & show better error (#19828)
Browse files Browse the repository at this point in the history
* Unify context data
* Actually show invalid url in error
  • Loading branch information
Gusted authored Jun 12, 2022
1 parent 3898fc5 commit edf1420
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 14 deletions.
2 changes: 1 addition & 1 deletion modules/web/middleware/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func Validate(errs binding.Errors, data map[string]interface{}, f Form, l transl
case binding.ERR_EMAIL:
data["ErrorMsg"] = trName + l.Tr("form.email_error")
case binding.ERR_URL:
data["ErrorMsg"] = trName + l.Tr("form.url_error")
data["ErrorMsg"] = trName + l.Tr("form.url_error", errs[0].Message)
case binding.ERR_INCLUDE:
data["ErrorMsg"] = trName + l.Tr("form.include_error", GetInclude(field))
case validation.ErrGlobPattern:
Expand Down
2 changes: 1 addition & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ size_error = ` must be size %s.`
min_size_error = ` must contain at least %s characters.`
max_size_error = ` must contain at most %s characters.`
email_error = ` is not a valid email address.`
url_error = ` is not a valid URL.`
url_error = `'%s' is not a valid URL.`
include_error = ` must contain substring '%s'.`
glob_pattern_error = ` glob pattern is invalid: %s.`
regex_pattern_error = ` regex pattern is invalid: %s.`
Expand Down
6 changes: 3 additions & 3 deletions routers/web/repo/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func handleMigrateRemoteAddrError(ctx *context.Context, err error, tpl base.TplN
case addrErr.IsProtocolInvalid:
ctx.RenderWithErr(ctx.Tr("repo.mirror_address_protocol_invalid"), tpl, form)
case addrErr.IsURLError:
ctx.RenderWithErr(ctx.Tr("form.url_error"), tpl, form)
ctx.RenderWithErr(ctx.Tr("form.url_error", addrErr.Host), tpl, form)
case addrErr.IsPermissionDenied:
if addrErr.LocalPath {
ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied"), tpl, form)
Expand All @@ -139,11 +139,11 @@ func handleMigrateRemoteAddrError(ctx *context.Context, err error, tpl base.TplN
ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tpl, form)
default:
log.Error("Error whilst updating url: %v", err)
ctx.RenderWithErr(ctx.Tr("form.url_error"), tpl, form)
ctx.RenderWithErr(ctx.Tr("form.url_error", "unknown"), tpl, form)
}
} else {
log.Error("Error whilst updating url: %v", err)
ctx.RenderWithErr(ctx.Tr("form.url_error"), tpl, form)
ctx.RenderWithErr(ctx.Tr("form.url_error", "unknown"), tpl, form)
}
}

Expand Down
12 changes: 7 additions & 5 deletions routers/web/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ const (
tplProtectedBranch base.TplName = "repo/settings/protected_branch"
)

// Settings show a repository's settings page
func Settings(ctx *context.Context) {
// SettingsCtxData is a middleware that sets all the general context data for the
// settings template.
func SettingsCtxData(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.settings")
ctx.Data["PageIsSettingsOptions"] = true
ctx.Data["ForcePrivate"] = setting.Repository.ForcePrivate
Expand Down Expand Up @@ -94,15 +95,16 @@ func Settings(ctx *context.Context) {
return
}
ctx.Data["PushMirrors"] = pushMirrors
}

// Settings show a repository's settings page
func Settings(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplSettingsOptions)
}

// SettingsPost response for changes of a repository
func SettingsPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.RepoSettingForm)
ctx.Data["Title"] = ctx.Tr("repo.settings")
ctx.Data["PageIsSettingsOptions"] = true

ctx.Data["ForcePrivate"] = setting.Repository.ForcePrivate
ctx.Data["MirrorsEnabled"] = setting.Mirror.Enabled
Expand Down Expand Up @@ -827,7 +829,7 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R
case addrErr.IsProtocolInvalid:
ctx.RenderWithErr(ctx.Tr("repo.mirror_address_protocol_invalid"), tplSettingsOptions, form)
case addrErr.IsURLError:
ctx.RenderWithErr(ctx.Tr("form.url_error"), tplSettingsOptions, form)
ctx.RenderWithErr(ctx.Tr("form.url_error", addrErr.Host), tplSettingsOptions, form)
case addrErr.IsPermissionDenied:
if addrErr.LocalPath {
ctx.RenderWithErr(ctx.Tr("repo.migrate.permission_denied"), tplSettingsOptions, form)
Expand Down
6 changes: 4 additions & 2 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,10 @@ func RegisterRoutes(m *web.Route) {

m.Group("/{username}/{reponame}", func() {
m.Group("/settings", func() {
m.Combo("").Get(repo.Settings).
Post(bindIgnErr(forms.RepoSettingForm{}), repo.SettingsPost)
m.Group("", func() {
m.Combo("").Get(repo.Settings).
Post(bindIgnErr(forms.RepoSettingForm{}), repo.SettingsPost)
}, repo.SettingsCtxData)
m.Post("/avatar", bindIgnErr(forms.AvatarForm{}), repo.SettingsAvatar)
m.Post("/avatar/delete", repo.SettingsDeleteAvatar)

Expand Down
2 changes: 1 addition & 1 deletion services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func ParseRemoteAddr(remoteAddr, authUsername, authPassword string) (string, err
strings.HasPrefix(remoteAddr, "git://") {
u, err := url.Parse(remoteAddr)
if err != nil {
return "", &models.ErrInvalidCloneAddr{IsURLError: true}
return "", &models.ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr}
}
if len(authUsername)+len(authPassword) > 0 {
u.User = url.UserPassword(authUsername, authPassword)
Expand Down
2 changes: 1 addition & 1 deletion services/migrations/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func IsMigrateURLAllowed(remoteURL string, doer *user_model.User) error {
// Remote address can be HTTP/HTTPS/Git URL or local path.
u, err := url.Parse(remoteURL)
if err != nil {
return &models.ErrInvalidCloneAddr{IsURLError: true}
return &models.ErrInvalidCloneAddr{IsURLError: true, Host: remoteURL}
}

if u.Scheme == "file" || u.Scheme == "" {
Expand Down

0 comments on commit edf1420

Please sign in to comment.