go-ozzo / ozzo-validation

An idiomatic Go (golang) validation package. Supports configurable and extensible validation rules (validators) using normal language constructs instead of error-prone struct tags.
MIT License
3.73k stars 224 forks source link

Add translation feature. #91

Closed mehran-prs closed 4 years ago

mehran-prs commented 4 years ago

Hi, I added the translation feature and also modified all the rules to use this feature.

I assume each rule's message property is the custom message, so any user sets it, that rule returns the user's message, otherwise, return the translated message.

Each rule can return the translated message if it does not exists return the English message, and if the English message does not exist for that rule, return rule's default message.

All functions work just like before Except one function:
I Changed NewStringRule function signature from
func validation.NewStringRule(validation stringValidator,message string) to
func validation.NewStringRule(validation stringValidator,ruleName string)
this is because we don't need the message here anymore, just need to get a name for that rule and get the rule's message from the translation map.

I also changed your package version from 3 to 4 because of NewStringRule function's signature changes.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling df2b2c7dd2440fd39db4e099841a0aa736758431 on mehran-prs:master into f7b74469335f4d2ebadd913e29277b57142c902b on go-ozzo:master.

qiangxue commented 4 years ago

Thank you for your attempt to add message translation support! I noticed a global variable Lang is used to determine which language should be used. If the validation error messages are meant to be shown to end users (that's also the main reason why translation is wanted), the language in use would be determined by the request context. A global language setting would not work in this scenario.

mehran-prs commented 4 years ago

you're welcome qiangxue,

OK, Each rule can return ValidationErr that is something like this:

type ErrMessage struct {
        Lang           string
        TranslationKey string
        DefaultMessage string
        Message        string
        Params         []interface{}
}

func (e *ErrMessage) Lang(lang string) {
    e.lang = lang
}

func (e ErrMessage) Error() string {
    msg := MsgInLang(lang, e.translationKey, e.defaultMessage, e.message)
    return fmt.Sprintf(msg, e.translationParams...)
}

and finally, after validating our data, we can set lang on erros and get translation in any language that we want.

Are you ok with this changes?

mehran-prs commented 4 years ago

Hi @qiangxue

everything works the same as before and we have feature-rich validation errors now :)

mehran-prs commented 4 years ago

@qiangxue can you see my PR, please?

qiangxue commented 4 years ago

@mehran-prs I have reviewed your code again. I really like the idea of enhancing the validation error definition so that it supports translation. However, I think i18n/translation doesn't belong to this library. There are existing libraries doing this.

Here's what I'm thinking:

I'd like to hear your comments. Also, before you spend time creating a major PR, can we discuss about the design first? I feel bad when denying a PR like this because I know you have spent a lot of time on it. Thank you!

mehran-prs commented 4 years ago

You're right @qiangxue, it's better to have a translator interface instead of implementing i18n/translator. I had to talk to you before creating PR. go-i18n seems to be the most used translator package in Go and I gave it a try.

In addition to error struct fields that you said, I think it should also have:

I think something like this can be good for us:

// Translate method on ErrMessage: func (e ErrMessage) Translate(t Translator) (string, error) { return t.TranslateSingleFieldErr(e) }


* `Translate` method on `Errors`
```go
func (e Errors) Translate(t Translator) (string, error) {
    // use t.TranslateStructFieldErr(field string,err) on each error
}

Are you ok with something like this?

qiangxue commented 4 years ago

I'm thinking about an even simpler solution which is still flexible enough to support i18n.

First define an Error struct:

type Error struct {
    code string
    message string
    params map[string]interface{}
}

The Error struct implements the error interface and also the methods exposing its fields. This definition should be sufficient for representing most error messages that need to be translated.

For each validation rule, modify its Error() method so that it can also accept an error. The default error messages for the rules are represented using the Error struct so that they can be translated, if needed.

For error messages that need more complex translation rules, users can create their own error types and pass them into the rules.

mehran-prs commented 4 years ago

OK, So I implement it.

mehran-prs commented 4 years ago

@qiangxue I updated my PR. simple map in messages is just for the package's default messages, this keeps the package cleaner.

If you think my repo needs to squash commits, just say.

qiangxue commented 4 years ago

Great job! Thank you very much for your contribution!

mehran-prs commented 4 years ago

you're welcome Qiangxue :)