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/descriptive validation messages #371

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 20 additions & 1 deletion validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"regexp"
"runtime"
"unicode"
)

// Simple struct to store the Message & Key of a validation error
Expand Down Expand Up @@ -153,7 +154,7 @@ func (v *Validation) apply(chk Validator, obj interface{}) *ValidationResult {

// Add the error to the validation context.
err := &ValidationError{
Message: chk.DefaultMessage(),
Message: fmt.Sprintf("%s %s", formatFieldName(key), chk.DefaultMessage()),
Key: key,
}
v.Errors = append(v.Errors, err)
Expand Down Expand Up @@ -243,6 +244,24 @@ func restoreValidationErrors(req *http.Request) ([]*ValidationError, error) {
return errors, err
}

// Format field name for display
func formatFieldName(field string) (formattedField string) {
if field == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's more idiomatic to do checks and return early rather than nesting large blocks of code.

For example

if field == "" {
  return "Field"
}

...

Copy link
Member Author

Choose a reason for hiding this comment

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

I see what you're saying, but what are you basing your measure of what's idiomatic for Go?

Copy link
Contributor

Choose a reason for hiding this comment

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

What everyone on the golang-nuts mailing list says.

It's not the easiest thing to search for, but here's one reference:
http://talks.golang.org/2013/bestpractices.slide

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I'll get my code in line with the current best practices sometime this week, unless you'd rather scrap this PR for now in favor of reworking the validation system.

formattedField = "Field"
} else {
var words []rune
for _, character := range field {
if unicode.IsUpper(character) {
words = append(words, rune(' '))
}
words = append(words, unicode.ToLower(rune(character)))
}
words[0] = unicode.ToUpper(words[0])
formattedField = string(words)
}
return
}

// Register default validation keys for all calls to Controller.Validation.Func().
// Map from (package).func => (line => name of first arg to Validation func)
// E.g. "myapp/controllers.helper" or "myapp/controllers.(*Application).Action"
Expand Down
14 changes: 14 additions & 0 deletions validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,17 @@ func TestValidationNoKeepCookiePreviouslySet(t *testing.T) {
t.Fatalf("cookie should be deleted")
}
}

// Test that field names can be properly formatted for validation error messages
func TestFieldNameFormatting(t *testing.T) {
fieldAscii := "someField"
expectedAscii := "Some field"
fieldUnicode := "sömeϜield"
expectedUnicode := "Söme ϝield"
if formattedAscii := formatFieldName(fieldAscii); formattedAscii != expectedAscii {
t.Errorf("ASCII field formatting failed. Expecting %s, got %s", expectedAscii, formattedAscii)
}
if formattedUnicode := formatFieldName(fieldUnicode); formattedUnicode != expectedUnicode {
t.Errorf("ASCII field formatting failed. Expecting %s, got %s", expectedUnicode, formattedUnicode)
}
}
18 changes: 9 additions & 9 deletions validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (r Required) IsSatisfied(obj interface{}) bool {
}

func (r Required) DefaultMessage() string {
return "Required"
return "is required"
}

type Min struct {
Expand All @@ -55,7 +55,7 @@ func (m Min) IsSatisfied(obj interface{}) bool {
}

func (m Min) DefaultMessage() string {
return fmt.Sprintln("Minimum is", m.Min)
return fmt.Sprintln("minimum is", m.Min)
}

type Max struct {
Expand All @@ -71,7 +71,7 @@ func (m Max) IsSatisfied(obj interface{}) bool {
}

func (m Max) DefaultMessage() string {
return fmt.Sprintln("Maximum is", m.Max)
return fmt.Sprintln("maximum is", m.Max)
}

// Requires an integer to be within Min, Max inclusive.
Expand All @@ -85,7 +85,7 @@ func (r Range) IsSatisfied(obj interface{}) bool {
}

func (r Range) DefaultMessage() string {
return fmt.Sprintln("Range is", r.Min.Min, "to", r.Max.Max)
return fmt.Sprintln("range is", r.Min.Min, "to", r.Max.Max)
}

// Requires an array or string to be at least a given length.
Expand All @@ -105,7 +105,7 @@ func (m MinSize) IsSatisfied(obj interface{}) bool {
}

func (m MinSize) DefaultMessage() string {
return fmt.Sprintln("Minimum size is", m.Min)
return fmt.Sprintln("minimum size is", m.Min)
}

// Requires an array or string to be at most a given length.
Expand All @@ -125,7 +125,7 @@ func (m MaxSize) IsSatisfied(obj interface{}) bool {
}

func (m MaxSize) DefaultMessage() string {
return fmt.Sprintln("Maximum size is", m.Max)
return fmt.Sprintln("maximum size is", m.Max)
}

// Requires an array or string to be exactly a given length.
Expand All @@ -145,7 +145,7 @@ func (s Length) IsSatisfied(obj interface{}) bool {
}

func (s Length) DefaultMessage() string {
return fmt.Sprintln("Required length is", s.N)
return fmt.Sprintln("required length is", s.N)
}

// Requires a string to match a given regex.
Expand All @@ -159,7 +159,7 @@ func (m Match) IsSatisfied(obj interface{}) bool {
}

func (m Match) DefaultMessage() string {
return fmt.Sprintln("Must match", m.Regexp)
return fmt.Sprintln("must match", m.Regexp)
}

var emailPattern = regexp.MustCompile("[\\w!#$%&'*+/=?^_`{|}~-]+(?:\\.[\\w!#$%&'*+/=?^_`{|}~-]+)*@(?:[\\w](?:[\\w-]*[\\w])?\\.)+[a-zA-Z0-9](?:[\\w-]*[\\w])?")
Expand All @@ -169,5 +169,5 @@ type Email struct {
}

func (e Email) DefaultMessage() string {
return fmt.Sprintln("Must be a valid email address")
return fmt.Sprintln("must be a valid email address")
}