Holding the values in form when the post request is failed with error

[yp05327]

Description

At first, I found that some vaules in the form were missing or changed to default values automaticly, when creating an org or a repo with invalid name.
e.g.

Visibility in this page is required so users will be aware of the change.
But if there are some no required params with this bug, user may post wrong data with no awareness of these changes.

Then I found func AssignForm in modules/web/middleware/binding.go which is used to assign form values back to the template data.
It seems we need to define same variable names between form data and template data?

e.g.
Visibility in this page is defined as DefaultOrgVisibilityMode in template data, but visibility in form data.

Gitea Version

87261f3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

build

Database

None

[yp05327]

It seems that we don't have the same naming convention to these variables.
e.g. variables in context data have three types of naming convention in this function.

gitea/routers/web/repo/repo.go

Lines 130 to 157 in 29aea36

	func Create(ctx *context.Context) {
	ctx.Data["Title"] = ctx.Tr("new_repo")
	// Give default value for template to render.
	ctx.Data["Gitignores"] = repo_module.Gitignores
	ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
	ctx.Data["Licenses"] = repo_module.Licenses
	ctx.Data["Readmes"] = repo_module.Readmes
	ctx.Data["readme"] = "Default"
	ctx.Data["private"] = getRepoPrivate(ctx)
	ctx.Data["IsForcedPrivate"] = setting.Repository.ForcePrivate
	ctx.Data["default_branch"] = setting.Repository.DefaultBranch
	ctxUser := checkContextUser(ctx, ctx.FormInt64("org"))
	if ctx.Written() {
	return
	}
	ctx.Data["ContextUser"] = ctxUser
	ctx.Data["repo_template_name"] = ctx.Tr("repo.template_select")
	templateID := ctx.FormInt64("template_id")
	if templateID > 0 {
	templateRepo, err := repo_model.GetRepositoryByID(ctx, templateID)
	if err == nil && access_model.CheckRepoUnitUser(ctx, templateRepo, ctxUser, unit.TypeCode) {
	ctx.Data["repo_template"] = templateID
	ctx.Data["repo_template_name"] = templateRepo.Name
	}
	}

To fix this issue, I think a naming convention is required. Then make some small changes to AssignForm if it is necessary.

e.g.

For all variables in context data, capitalizing the first letter of each word and join them with _, just like: Default_Words
For all variables in form data, lower-case words and join them with _, just like: default_words
And adding Formor Param before each variable's name is better, because it is easy to know whether the variable is used in form data.
Then add an exchanging name process in AssignForm.

[wxiaoguang]

I could agree with you (at least partially)

In history, people come and go, no strict rules nor guidelines, various code gets merged, many framework designs and mechanisms get broken ....

So, a strong (long-term) team with strict and reliable review workflow could help, that's a trade-off. I think things can get better.

[yp05327]

The document is renewing under https://docs.gitea.com/.
Maybe it is a good chance to supplement develop guidelines.

I will try to make a preview PR which will fix this in a better way.

[wolfogre]

I noticed that #22834 and #22846 are both trying to fix it too, but in different ways. #22834 is a quick patch.

So shall we merge #22834 first and wait for #22846 to finish?

[yp05327]

I noticed that #22834 and #22846 are both trying to fix it too, but in different ways. #22834 is a quick patch.

So shall we merge #22834 first and wait for #22846 to finish?

@wolfogre
No problem.