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

Naming property Length and using IsNumber isn't valid #167

Closed
fehrm opened this issue Aug 9, 2017 · 3 comments
Closed

Naming property Length and using IsNumber isn't valid #167

fehrm opened this issue Aug 9, 2017 · 3 comments
Labels

Comments

@fehrm
Copy link

fehrm commented Aug 9, 2017

If I name a property Length, and then use IsNumber inside AssertThat on it, like this:
https://github.com/fehrm/LengthEA/blob/d9839bdb3e70723c236b9cbadec97680348c4853/LengthEA/Models/TestViewModel.cs#L11-L12

[AssertThat("IsNumber(Length)")]
public string Length { get; set; }

It doesn't validate it correctly in the clientvalidation when validated. If I rename the property to eg. LengthT it works.

I added a sample project to my github, https://github.com/fehrm/LengthEA/ with a very simple testcase for it. It behaves the same if I don't use input type number.

This is using EA 2.9.2, jQuery valid 1.16.0 etc. Here is a link to the packages.config

@jwaliszko jwaliszko added the bug label Aug 9, 2017
@jwaliszko
Copy link
Owner

jwaliszko commented Aug 9, 2017

Hi,

The problem is due to the Length property being overridden by a built-in method of the same name, when model context (within which expression is to be evaluated) is being constructed.
Proposed fix here: f39e431.

Thanks for raising this defect.

@fehrm
Copy link
Author

fehrm commented Aug 10, 2017

I agree that it is not an optimal name 😄 I think the proposed change is great though, it will enable the developer to be informed and change the name to a more suitable one. As I did for my project.

Thanks for the prompt reply!

@fehrm fehrm closed this as completed Aug 10, 2017
@jwaliszko
Copy link
Owner

jwaliszko commented Aug 12, 2017

The name is as good as any other, it's just EA which failed to handle it.
I've just improved the fix here: b2bd4f6.

This one is able to handle your original naming as expected - without any warnings and changes at your side. With this fix, only the essential set of methods, actually used in expression, is registered within model context. Previously all available methods were added to the context - hence bigger chance to have naming conflicts.

Finally, if naming conflict with some method actually occurs, e.g. in expression like Length(Length), exception is thrown as client-side compatibility is broken.

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

No branches or pull requests

2 participants