offirgolan / ember-changeset-cp-validations

Ember CP Validations support for Ember Changeset
MIT License
17 stars 10 forks source link

confirmation validations in component #29

Open olenderhub opened 6 years ago

olenderhub commented 6 years ago

Hello, I have an issue related to validate confirmation of real-time update (skip validate option). I created a special repository to show what is happening with instructions how to reproduce it. https://github.com/olenderhub/changeset-with-cp-validations

Should I use these addons in a different way or it's bug that needs to be fixed?

olenderhub commented 6 years ago

I just realized, that if I change in my validations on: 'password'

to on: 'changeset.password' it works properly, but should it work this way?

olenderhub commented 6 years ago

This is problematic using on: 'changeset.password', because I also use https://github.com/jasonmit/ember-i18n-cp-validations

and I have the error: "This field doesn't match changeset.password" with default translation: confirmation: "{{description}} doesn't match {{on}}",

olenderhub commented 6 years ago

I tried too create model from EmberObject, but it's also impossible because of https://github.com/offirgolan/ember-changeset-cp-validations/blob/b833618eb42ae4296f65ac582dc423dbc2a452f7/addon/index.js#L9


let { validateFn, validationMap } = buildChangeset(model);```

@offirgolan Do you have any suggestions about this case?
olenderhub commented 6 years ago

Sorry for spam, but still trying to do something.

I created const model = Ember.Object.extend(Validations).create(Ember.getOwner(this).ownerInjection()); and used

this.changeset = createChangeset(model); (from import createChangeset from 'ember-changeset-cp-validations';)

on my confirm validation:

passwordConfirmation: [
    validator('presence', {
      presence: true
     }),
    validator('confirmation', {
      on: 'password'
     })
]

it's impossible to match password from changeset

I have value changeset.password and changeset.passwordConfirmation

and here: https://github.com/offirgolan/ember-validators/blob/5c88dbb31dead644ad333543a909a2a5678e6728/addon/confirmation.js#L21

I have current value from changeset.passwordConfirmation and the model, which doesn't have changes, because I change changeset instead of orginal model.

If I change in template it for example to model.password and model.passwordConfirmation, changeset will not observe automatically changes and even I fire changeset.validate(), I will have value = undefined.

cah-brian-gantzler commented 6 years ago

Just discovered this myself. The real problem is that the object that is passed into the validation is the actual model, not the changeset model (Meaning 'changeset.password' should NOT have worked for you).

I am guessing that the only validator that actually uses the model is confirmation and possibly custom.

I would think that changing the code to ensure that the changeset model is whats passed would solve the problem.

Looking into it. Although I can only do a pull request, Since no one answered your question yet, not sure if there is someone that would execute the pull request.

cah-brian-gantzler commented 6 years ago

After spending time looking at this issue, I do not see any way this could work without completely rewriting the cp validations addon. A reference to the model to be validated seems to be stored on every sub class. Many of the classes are not exposed to extend or reopen.

When I tried to override some of the functionality supporting functions needed were not exported (they were private and I understand why, but....)

I was able to change the model by some clever use of bind, but in cp-validations when it tried to perform from methods, it expected those methods to be on the object, and since changeset is a proxy, they were not found. I tried to go the route of exposing proxy methods on changeset, which also led to a lot of code that needed changed.

Im at a loss and just dont see this happening sadly.

cah-brian-gantzler commented 6 years ago

Ok, so here is the solution I came up with

  1. Create a validation mixin as normal contain only the confirm password validation. For the on use "model.password"
  2. Create a new Ember object, mixin the mixin from step one (do not make this a change set), assign the changeset model to the model property of this object
  3. wire up the input and error messages for the confirm password to the new object not the user changeset.

Yes you have to check isValid on both objects (a simple computed.and will abstract that) and hey, you really didnt need the property confirm password on the user object anyway did you.

Hope this helps