Core: Adding a way to pass method name to remote #1657

wojwal · 2015-12-01T17:27:31Z

This allows reusing remote as custom method via addMethod. Without this fix it will show standard "remote" message - instead of custom message from addMethod.

Sample usage:

    $.validator.addMethod('workemail', function(value, element, param) {
        return $.validator.methods.remote.call(this, value, element, {
            url: "validate/workemail/"
        }, 'workemail');
    }, 'work email custom message');

And in HTML I'm just using data-rule-workemail='true' attribute for input

wojwal · 2016-01-13T15:49:38Z

Hi, just wanted to get an update on this pull-request? :)

staabm · 2016-01-13T19:04:47Z

@wojwal thx for the PR. It somehow slipped thru :-)

I like it. Could you rebase it and add unit tests?

Arkni · 2016-01-13T19:14:16Z

I'm not against this approach, but can't we achieve the same result if we use: data-rule-remote="validate/workemail/" and data-msg-remote="work email custom message" ?

Please correct me if I misunderstood something.

wojwal · 2016-01-14T09:46:55Z

@staabm - I just rebased the code - so that part is ready. As for test units - I reviewed "test" folder, but it will take me some time to understand, how to do it properly - I'll do it when I got some spare time.

@Arkani - Not sure I guess your solution will work, but there is much more you can do with solution I proposed. For example:

you can have multiple custom methods in one page that uses remote - I think in the way you suggest you can have only 1 remote method with 1 defined message?
you can pass custom parameters as "data" to XHR requet
I use Javascript localization - so error message I passed to validator is additionally translated via Javascript - you wouldn't be able to do it with data-msg-remote attribute
HTML-wise - it's simpler just to have single attribute: "data-rule-workemail='params'" ;-)

I guess it's always better to have multiple possibilities ;)

Arkni · 2016-01-14T13:49:02Z

So your idea is using the remote rule inside other custom rules and use the error message of your rule as the one for the remote method. I like it :)

Feel free to tag me whenever you need any help regarding the unit test :)

By the way my username is @Arkni :D

Arkni · 2016-01-14T13:57:25Z

src/core.js

 			if ( this.optional( element ) ) {
 				return "dependency-mismatch";
 			}

-			var previous = this.previousValue( element ),
+			method = typeof method === "string" ? method : "remote";


Maybe this can be changed to:

method = method || "remote";

wojwal · 2016-01-17T10:24:05Z

Hi guys ;)

I've added tests for this, which includes:

checking error message passed to addMethod
additionally passing & requiring special parameter to XHR via data-rule-workemail attribute

@Arkni - as for your suggestion - I'm fine with either case. Your suggestion is just little less strict (not checking if value is string) - but thats not a big deal...

Arkni · 2016-01-17T13:42:08Z

as for your suggestion - I'm fine with either case. Your suggestion is just little less strict (not checking if value is string) - but thats not a big deal...

I know, but what will happen if a user passed an empty String (in a test for example) ?
So for a strict check, you could use:

method = typeof method === "string" && method || "remote";

Otherwise, LGTM :)

wojwal · 2016-01-18T08:00:14Z

I just commited that change ;)

wojwal · 2016-01-22T10:10:25Z

HI guys, pls merge my changes... Anything missing?

Thanks

staabm · 2016-01-28T17:43:44Z

@wojwal please rebase the branch against master. After I merge some other PRs today it is no longer mergeable. Will merge after your rebase ASAP. thanks.

This allows reusing remote as custom method via addMethod

wojwal · 2016-01-29T15:08:51Z

@staab rebased ;)

staabm · 2016-02-01T08:13:41Z

@wojwal thx

Maks3w · 2016-06-04T19:17:44Z

Just note this introduce a BC Break for those developers with custom validation functions.

This should be highlighted in the CHANGELOG

Arkni · 2016-06-04T22:36:27Z

Just note this introduce a BC Break ...

How so ?

Maks3w · 2016-06-05T05:56:58Z

If the additional argument is not provided then method is undefined

When {method: undefined} is sent to defaultMessage app blow up because cannot do method.charAt

Maks3w · 2016-06-05T05:57:59Z

So if previousValue method is not set then the value should fallback to "remote"

staabm · 2016-06-05T12:03:12Z

@Maks3w please open a new issue

Arkni · 2016-06-05T12:09:45Z

If the additional argument is not provided then method is undefined

No, that's not true. If the argument is not provided, then method will be set to remote prior to any call to previousValue, see https://github.com/jzaefferer/jquery-validation/blob/73c173198c2ab225ecb0713c55f509b30acbb9c0/src/core.js#L1426-L1428

Maks3w · 2016-06-05T15:29:07Z

Just note this introduce a BC Break for those developers with custom validation functions.

Arkni · 2016-06-06T17:56:59Z

Sorry, but I still didn't get it.

Maybe you can open an issue and provide a simplified example (in jsfiddle or something else).
Thanks!

Maks3w · 2016-06-06T18:02:13Z

No need of jsfiddle previousValue is public and their signature must not change in unsafe way.

publicFunction(oldArg, newArg) { if (newArg === undefined) newArg = defaultValue }

Arkni · 2016-06-06T19:19:34Z

Ah, I see. Thanks for the explanation.

This should be fixed as soon as possible, minor releases should never introduce breaking changes.

staabm · 2016-06-11T13:46:59Z

@Arkni I would like to release 15.1 next week, could you provide a fix for this until then?

Maks3w · 2016-06-11T13:49:44Z

One note defaultmessage has changed his behavior and the second argument must be an object. I'll put the note on the original PR if I found,

Maks3w · 2016-06-11T13:58:13Z

https://github.com/jzaefferer/jquery-validation/pull/1644/files#r66706223

Arkni · 2016-06-11T14:50:58Z

@Arkni I would like to release 15.1 next week, could you provide a fix for this until then?

Sure!

…ation#1657

…-validation#1657

As of now the signature of `validator.defaultMessage( element, string )` is deprecated. Use `validator.defaultMessage( element, object )` instead where objet is defined as ``` object = { method: "method name", parameters: "the given method parameters" } ```

wojwal force-pushed the master branch 2 times, most recently from 1d89469 to 8d96f00 Compare December 2, 2015 11:23

staabm added the NEEDS TEST label Jan 13, 2016

wojwal force-pushed the master branch from 8d96f00 to 5eb1017 Compare January 14, 2016 08:55

Arkni reviewed Jan 14, 2016
View reviewed changes

wojwal force-pushed the master branch 2 times, most recently from 22965c5 to 6345f73 Compare January 17, 2016 10:16

wojwal force-pushed the master branch from 6345f73 to 2e14abe Compare January 18, 2016 07:57

staabm removed the NEEDS TEST label Jan 28, 2016

Core: Adding a way to pass method name to remote

a9ae78e

This allows reusing remote as custom method via addMethod

wojwal force-pushed the master branch from 2e14abe to a9ae78e Compare January 29, 2016 15:01

staabm closed this in 4101b89 Feb 1, 2016

Arkni added a commit to Arkni/jquery-validation that referenced this pull request Jun 11, 2016

Fix regressions introduced in jquery-validation#1644 and jquery-valid…

7b9e987

…ation#1657

Arkni added a commit to Arkni/jquery-validation that referenced this pull request Jun 11, 2016

Fix regressions introduced in jquery-validation#1644 and jquery-valid…

d5e6d3f

…ation#1657

Arkni added a commit to Arkni/jquery-validation that referenced this pull request Jun 11, 2016

Core: Fix regressions introduced in jquery-validation#1644 and jquery…

eaf96db

…-validation#1657

Arkni added a commit to Arkni/jquery-validation that referenced this pull request Jun 14, 2016

Core: Fix regressions introduced in jquery-validation#1644 and jquery…

9d2f9ae

…-validation#1657

Arkni added a commit to Arkni/jquery-validation that referenced this pull request Jun 14, 2016

Core: Fix regressions introduced in jquery-validation#1644 and jquery…

986a47d

…-validation#1657

Uh oh!

Core: Adding a way to pass method name to remote #1657

Core: Adding a way to pass method name to remote #1657

Conversation

wojwal commented Dec 1, 2015

Uh oh!

wojwal commented Jan 13, 2016

Uh oh!

staabm commented Jan 13, 2016

Uh oh!

Arkni commented Jan 13, 2016

Uh oh!

wojwal commented Jan 14, 2016

Uh oh!

Arkni commented Jan 14, 2016

Uh oh!

Arkni Jan 14, 2016

Choose a reason for hiding this comment

Uh oh!

wojwal commented Jan 17, 2016

Uh oh!

Arkni commented Jan 17, 2016

Uh oh!

wojwal commented Jan 18, 2016

Uh oh!

wojwal commented Jan 22, 2016

Uh oh!

staabm commented Jan 28, 2016

Uh oh!

wojwal commented Jan 29, 2016

Uh oh!

staabm commented Feb 1, 2016

Uh oh!

Maks3w commented Jun 4, 2016

Uh oh!

Arkni commented Jun 4, 2016

Uh oh!

Maks3w commented Jun 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Maks3w commented Jun 5, 2016

Uh oh!

staabm commented Jun 5, 2016

Uh oh!

Arkni commented Jun 5, 2016

Uh oh!

Maks3w commented Jun 5, 2016

Uh oh!

Arkni commented Jun 6, 2016

Uh oh!

Maks3w commented Jun 6, 2016

Uh oh!

Arkni commented Jun 6, 2016

Uh oh!

staabm commented Jun 11, 2016

Uh oh!

Maks3w commented Jun 11, 2016

Uh oh!

Maks3w commented Jun 11, 2016

Uh oh!

Arkni commented Jun 11, 2016

Uh oh!

Uh oh!

Maks3w commented Jun 5, 2016 •

edited

Loading