Reworked Required and OmitEmpty #8

KN4CK3R · 2021-05-04T15:02:44Z

KN4CK3R commented

2021-05-04 15:02:44 +00:00

The old behaviour is inconsistent because this fails:

type TestForm struct {
	Valid string `binding:"Url"`
	Fails string `binding:"Email"`
}

form := &TestForm{
	Valid: "",
	Fails: "",
}
_ := RawValidate(form)

Here you need OmitEmpty even if the field is not required.

The new logic:

Empty	Required	Valid
Yes	Yes	No
Yes	No	Yes, all other rules are skipped
No	/	Maybe, other rules decide

In short: Rules are skipped if a field is empty and not required. Thats the normal behaviour in other validation libraries like Yup too.

After this change there are some places in Gitea where we can add form validation to optional fields which is not possible at the moment. Possible changes in:
21465a2ce3/modules/structs/admin_user.go (L31)
21465a2ce3/modules/structs/admin_user.go (L35)
21465a2ce3/modules/structs/org_team.go (L38)
21465a2ce3/modules/validation/binding.go (L90)
21465a2ce3/modules/validation/binding.go (L108)

The old behaviour is inconsistent because this fails: ```go type TestForm struct { Valid string `binding:"Url"` Fails string `binding:"Email"` } form := &TestForm{ Valid: "", Fails: "", } _ := RawValidate(form) ``` Here you need `OmitEmpty` even if the field is not required. The new logic: | Empty | Required | Valid | | -------- | -------- | -------- | | Yes | Yes | No | | Yes | No | Yes, all other rules are skipped | | No | / | Maybe, other rules decide | In short: Rules are skipped if a field is empty and not required. Thats the normal behaviour in other validation libraries like Yup too. After this change there are some places in Gitea where we can add form validation to optional fields which is not possible at the moment. Possible changes in: https://github.com/go-gitea/gitea/blob/21465a2ce361eeb2bff171a0b962dac771ab18d9/modules/structs/admin_user.go#L31 https://github.com/go-gitea/gitea/blob/21465a2ce361eeb2bff171a0b962dac771ab18d9/modules/structs/admin_user.go#L35 https://github.com/go-gitea/gitea/blob/21465a2ce361eeb2bff171a0b962dac771ab18d9/modules/structs/org_team.go#L38 https://github.com/go-gitea/gitea/blob/21465a2ce361eeb2bff171a0b962dac771ab18d9/modules/validation/binding.go#L90 https://github.com/go-gitea/gitea/blob/21465a2ce361eeb2bff171a0b962dac771ab18d9/modules/validation/binding.go#L108

KN4CK3R added 2 commits 2021-05-04 15:02:44 +00:00

Fail fast on zero values. 0b6d21f427

Added tests.

continuous-integration/drone/pr Build was killed

Details

e519a9d825

KN4CK3R added 1 commit 2021-05-05 06:25:00 +00:00

Merge branch 'master' of https://gitea.com/go-chi/binding into feature-ignore-empty

continuous-integration/drone/pr Build was killed

Details

705619b591

lunny commented

2021-05-05 10:03:40 +00:00

Please resolve the conflicts.

KN4CK3R commented

2021-05-05 10:20:56 +00:00

I see no conflicts? There was a conflict which should be resolved with the merge of the master branch some hours ago.

lunny commented

2021-06-09 01:08:09 +00:00

It maybe a bug of Gitea, when update, it displays below error.

Auto-merging validate_test.go
CONFLICT (content): Merge conflict in validate_test.go
Auto-merging binding.go
Automatic merge failed; fix conflicts and then commit the result.

It maybe a bug of Gitea, when update, it displays below error. ``` Auto-merging validate_test.go CONFLICT (content): Merge conflict in validate_test.go Auto-merging binding.go Automatic merge failed; fix conflicts and then commit the result. ```

6543 commented

2021-06-09 01:09:39 +00:00

@KN4CK3R can you rebase or update this pull please?

KN4CK3R added 1 commit 2021-06-09 13:13:09 +00:00

Merge branch 'master' of https://gitea.com/go-chi/binding into feature-ignore-empty

continuous-integration/drone/pr Build is passing

Details

792294b7cf

6543 approved these changes 2021-06-09 13:36:20 +00:00

lunny approved these changes 2021-06-09 15:31:55 +00:00

lunny merged commit b29e041704 into master

2021-06-09 15:32:02 +00:00