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

🔥 Feature: Add CustomTypeRegister for form #1076

Closed
wants to merge 9 commits into from

Conversation

rockcreation7
Copy link
Contributor

@rockcreation7 rockcreation7 commented Dec 14, 2020

Add "CustomTypeRegister" for form data bodyparser, allow you to input any custom type, provide flexibility for form, so you can register you own custom type in form.

Please provide enough information so that others can review your pull request:

I added "CustomTypeRegister" struct just above the bodyparser, and optional parameter for form bodyparser.
The comment above "CustomTypeRegister" provide example and how to use.

Explain the details for making this change. What existing problem does the pull request solve?

You can do custom type in JSON format, Create custom type and related "UnmarshalJSON" method.
But in form you need to register converter in decoder, I added option for supporting this feature. In order to solve problem when you need custom type for form.

For example:
It is mostly happen when people will sent you "2020-12-15" in browser form instead of RFC3339 format.
You can register this custom format, instead of force to use RFC3339. Other use case like, You can register own member ID type, etc.

Commit formatting

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

Add "CustomTypeRegister" for form data, allow you to input any custom type, provide flexibility for form, so you can register you own type in form.
@welcome
Copy link

welcome bot commented Dec 14, 2020

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

ctx.go Outdated Show resolved Hide resolved
@rockcreation7 rockcreation7 marked this pull request as draft December 15, 2020 13:05
@rockcreation7 rockcreation7 marked this pull request as ready for review December 15, 2020 13:09
add unit test for BodyParser
@rockcreation7
Copy link
Contributor Author

rockcreation7 commented Dec 16, 2020

I have provided a unit test with parameter, just under Test_Ctx_BodyParser, it could be doing in Test_Ctx_BodyParser, but it require extra condition and ReleaseCtx.

@Fenny
Copy link
Member

Fenny commented Dec 17, 2020

PR looks good, I'm just not sure about struct naming. Since it's a public method attached to Fiber, we won't be able to change it in the future.

What do you think about BodyParserType or BodyParserRegister?

@rockcreation7
Copy link
Contributor Author

Thanks, it is about adding type and eventually it is adding type to BodyParser, "BodyParserType" is related and descriptive. one consideration is bodyparser is also working on json, dont know if user will confuse/think BodyParserType will work on json too.

But I believe doc will explain, I plan to work on the doc on this too.

Btw the doc of fiber and recipe section is clear and good, it helps a lot to me.

@Fenny
Copy link
Member

Fenny commented Dec 22, 2020

After I saw #1084, maybe we should provide an option to override the schema decoder globally?

@rockcreation7
Copy link
Contributor Author

Will be update

rock added 3 commits December 25, 2020 14:47
🔥 Feature : SetBodyParserDecoder map all option to form decoder

♻️ Move BodyParserType to SetBodyParserDecoder

🚨 Test: Test_Ctx_BodyParser_WithBodyParserType rename to Test_Ctx_BodyParser_WithSetBodyParserDecoder
Updated to Test for ZeroEmpty
@rockcreation7
Copy link
Contributor Author

rockcreation7 commented Dec 30, 2020

More elaboration on the code:

Added Following function for override the schema decoder globally

customType := BodyParserType{
		Customtype: customType{},
		Converter:  timeConverter,
}

SetBodyParserDecoder(
         BodyParserConfig{
		IgnoreUnknownKeys: true,
		BodyParserType:    []BodyParserType{customType},
		ZeroEmpty:         true,
		SetAliasTag:       "form",
	}
)

The test is also update in last commit.

Please let me know if there any change/idea should be update for merging.
Enjoy Holidays guys

@rockcreation7 rockcreation7 marked this pull request as draft January 2, 2021 19:58
@rockcreation7
Copy link
Contributor Author

Closing this branch, duplication, new branch with bigger scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants