Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify repo settings & show better error #19828

Merged
merged 9 commits into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
zeripath marked this conversation as resolved.
Show resolved Hide resolved
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