Skip to content

feat: [#416] Support add custom filter for validation#516

Merged
krishankumar01 merged 8 commits intomasterfrom
kkumar-gcc/#416
Jun 27, 2024
Merged

feat: [#416] Support add custom filter for validation#516
krishankumar01 merged 8 commits intomasterfrom
kkumar-gcc/#416

Conversation

@krishankumar01
Copy link
Member

📑 Description

Closes goravel/goravel#416

✅ Checks

  • Added test cases for my code

@krishankumar01 krishankumar01 requested a review from a team June 25, 2024 05:38
@codecov
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.98%. Comparing base (0ad5442) to head (ce15251).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
+ Coverage   69.90%   69.98%   +0.07%     
==========================================
  Files         180      180              
  Lines       11000    11025      +25     
==========================================
+ Hits         7690     7716      +26     
+ Misses       2739     2738       -1     
  Partials      571      571              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

devhaozi
devhaozi previously approved these changes Jun 25, 2024
// Make create a new validator instance.
Make(data any, rules map[string]string, options ...Option) (Validator, error)
// AddFilter add the custom filter.
AddFilter(name string, filterFunc any) Validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to define a filter interface and pass it here. Like the rule:

image

Add a command to create a filter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's not a good idea since the filter function can vary.
image

Copy link
Contributor

@hwbrzzl hwbrzzl Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

type Filter interface {
	// Signature set the unique signature of the filter.
	Signature() string
	// Passes determine if the validation rule passes.
	Handle() func(val any) (any, error)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's not a good idea since the filter function can vary.

We can only support one style: func(val any) (any, error)

Copy link
Member Author

@krishankumar01 krishankumar01 Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A developer can also create a filter function like this:

func Default(val string, def ...string) string {
    ...
}

which can be used like this:

map[string]string{
   "name" : "default:krishan"
}
// assume registered as "default"

So, let's not add constraints on developers since gookit/validate provides this feature.

Copy link
Member Author

@krishankumar01 krishankumar01 Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

type Filter interface {
	// Signature set the unique signature of the filter.
	Signature() string
	// Passes determine if the validation rule passes.
	Handle() func(val any) (any, error)
}

we can change it to ?

type Filter interface {
	// Signature set the unique signature of the filter.
	Signature() string
	// Passes determine if the validation rule passes.
	Handle() any // will mention in docs that any can only be two type of function
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to add a new Filter method in the FormRequest:

image

Users can use the Filter feature through the Filter method, instead of set a tag in the struct:

image

The way of set a tag in the struct is inconsistent with the rule, so I suggest optimize it. cc @devhaozi

So we need to fdmovd the logic in goravel/gin and goravel/fiber and add some new logic:

image

devhaozi
devhaozi previously approved these changes Jun 26, 2024
Copy link
Member

@devhaozi devhaozi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

hwbrzzl
hwbrzzl previously approved these changes Jun 27, 2024
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR 👍 Please add some integration tests in goravel/example

// Signature sets the unique signature of the filter.
Signature() string

// Handle defines the filter function to apply.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

Co-authored-by: Wenbo Han <hwbrzzl@gmail.com>
@krishankumar01 krishankumar01 dismissed stale reviews from hwbrzzl and devhaozi via 783a586 June 27, 2024 08:09
@krishankumar01 krishankumar01 enabled auto-merge (squash) June 27, 2024 08:13
@krishankumar01 krishankumar01 merged commit 612c188 into master Jun 27, 2024
@krishankumar01 krishankumar01 deleted the kkumar-gcc/#416 branch June 27, 2024 08:24
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.

✨ [Feature] Support add custom filter for validation

3 participants