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

Field error binding should not be hardcoded #3

Closed
vladvel opened this issue Oct 4, 2013 · 8 comments
Closed

Field error binding should not be hardcoded #3

vladvel opened this issue Oct 4, 2013 · 8 comments
Milestone

Comments

@vladvel
Copy link

vladvel commented Oct 4, 2013

Field error handler binds error to 'version' field of domain class in any case. I have some problems with i18n of error message, because I can not use i18n label for domain class. Also I can not use custom error message code or message for specific domain. In some cases error binding to version field is not needed, when specific handler is used.
I offer to add a config option to disable this behavior if needed, or disable it when onConflict handler is being used.

@nobeans
Copy link
Member

nobeans commented Oct 4, 2013

It sounds good. there are three ways:

  1. using a default message (already supported)
  2. using a custom message code and args and a default message (args and a default message can be omitted)
  3. not using a field error

you need 2 and 3, don't you?

@nobeans
Copy link
Member

nobeans commented Oct 4, 2013

These behave as same as before, binding the default field error with hard-coded message code:

myDomain.withOptimisticLock { domain ->
    domain.save()
}

OR

myDomain.withOptimisticLock { domain ->
    domain.save()
}.onConflict { domain, cause ->  // defaultErrorBinding:true by default
    ...
}

OR

myDomain.withOptimisticLock { domain ->
    domain.save()
}.onConflict(defaultErrorBinding: true) { domain, cause -> // explicitly specified
    ...
}

You can specified a parameter "defaultErrorBinding" with false to avoid binding default field error:

myDomain.withOptimisticLock { domain ->
    domain.save()
}.onConflict(defaultErrorBinding: false) { domain, cause ->
    domain.errors.rejectValue("your.code", [123] as Object[], "default!!")
    ...
}

How about it?

@vladvel
Copy link
Author

vladvel commented Oct 4, 2013

It is great! But can you also add configuration parameter to specify default behavior in Config.groovy? It will be setted to "true" by default (for backward compatibility) and used if parameter defaultErrorBinding in onConflict() handler is omitted.

@nobeans
Copy link
Member

nobeans commented Oct 5, 2013

It seems good to be able to specify default value in Config.groovy. okay, I'll try it.

@nobeans
Copy link
Member

nobeans commented Oct 5, 2013

I implemented that on a feature branch. Could you confirm the sample code as test cases?

https://github.com/kobo/grails-domain-locking/blob/issues/3/test/integration/org/jggug/kobo/domainlocking/OptimisticLockingDynamicMethodSpec.groovy#L153-L210

If you like it, I'll merged it to master branch.

@vladvel
Copy link
Author

vladvel commented Oct 5, 2013

The test cases are correct for my request. Thank you for this work! I am waiting for it to update my project.

@nobeans
Copy link
Member

nobeans commented Oct 6, 2013

Merged into master. v0.4 will be released asap. Thank you!

@nobeans nobeans closed this as completed Oct 6, 2013
@nobeans
Copy link
Member

nobeans commented Oct 8, 2013

v0.4 released. enjoy!

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

No branches or pull requests

2 participants