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

New Issue

GiteaMirror · 2025-11-02T09:01:55-06:00

GiteaMirror commented

2025-11-02 09:01:55 -06:00

Originally created by @yp05327 on GitHub (Feb 9, 2023).

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

87261f3fb9

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

Originally created by @yp05327 on GitHub (Feb 9, 2023). ### 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. ![image](https://user-images.githubusercontent.com/18380374/217716139-a1c54f61-2cd2-464c-a3e4-42f637ff6803.png) `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 87261f3fb95da92d74d72d057fc0f1e9819f16a7 ### 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

GiteaMirror added the type/bug label 2025-11-02 09:01:55 -06:00

GiteaMirror commented

2025-11-02 09:01:55 -06:00

@yp05327 commented on GitHub (Feb 9, 2023):

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.
29aea3642f/routers/web/repo/repo.go (L130-L157)

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.

@yp05327 commented on GitHub (Feb 9, 2023): 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. https://github.com/go-gitea/gitea/blob/29aea3642f5de5f1a8d4264f8360ddb5d072a861/routers/web/repo/repo.go#L130-L157 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 `Form`or `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`.

GiteaMirror commented

2025-11-02 09:01:56 -06:00

@wxiaoguang commented on GitHub (Feb 9, 2023):

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.

@wxiaoguang commented on GitHub (Feb 9, 2023): 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.

GiteaMirror commented

2025-11-02 09:01:56 -06:00

@yp05327 commented on GitHub (Feb 10, 2023):

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.

@yp05327 commented on GitHub (Feb 10, 2023): 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.

GiteaMirror commented

2025-11-02 09:01:56 -06:00

@wolfogre commented on GitHub (Feb 10, 2023):

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 commented on GitHub (Feb 10, 2023): 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?

GiteaMirror commented

2025-11-02 09:01:56 -06:00

@yp05327 commented on GitHub (Feb 10, 2023):

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.

@yp05327 commented on GitHub (Feb 10, 2023): > 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.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#10241