search

teamcfadvance / ValidateThis

An object oriented validation framework for ColdFusion objects
http://www.validatethis.org
44 stars 32 forks source link

injectResultIntoBO Can be defined when calling validate from Facade Object #79

Closed gregmoser closed 12 years ago

gregmoser commented 12 years ago

Set it up so the the injectResultIntoBO can be passed when you call the validate() method on the facade object. Perviously this had worked because the validateMethod was getting called with argumentcollection=arguments so the change that I had made against the ServerValidator was sufficient. However at some point that was changed to explicitly defining the arguments for each hand off.

bobsilverberg commented 12 years ago

Hey Gregg. Thanks for the pull request. I am not ignoring it, jut super busy. I will try to get to it this weekend.

gregmoser commented 12 years ago

No problem take your time... I'm going to try and look into that shared hosting permissions issue this weekend anyway. Hostek set us up with their standard sandboxed security environment as a testing setup so hopefully I'll be able to figure out what is going on.

aliaspooryorik commented 12 years ago

Hi Greg - as Bob is busy at the moment, I've had a look at your changes and they look great. I've run your code against the existing unit tests and it's all green. Would it be possible for you to add some unit tests to show that the Result object has been injected?

Thanks!

gregmoser commented 12 years ago

Hey John, I've got no problem writing a couple of unit tests for this however I'm not the most skilled unit test writer. The last time i did a pull request I went to add some unit tests, and i couldn't figure out how to execute or make them pass. Do you think you might have some time to give me a 10 min overview of how so that I can add them for all updates I want to submit to VT?

If not, I understand and I'll try to figure it out again on my own.

-Greg

aliaspooryorik commented 12 years ago

Hi Greg,

Here you go, I've added some comments so that hopefully it makes some sense: https://github.com/aliaspooryorik/ValidateThis/commit/62073c7a785f1717681b7ba8a0bb09d9a601036d

If you could add that to your pull request, that would be great :)

Any questions, let me know!

Cheers,

John

gregmoser commented 12 years ago

John, this is perfect... and exactly what i needed. I'm going to run this against my local, make sure it works and that i understand it, and then I'll add it to the pull request. Thanks again!

aliaspooryorik commented 12 years ago

You're welcome Greg - I ran it against my fork of your changes, but thought it would be easier to merge your changes into VT if it was in your pull request.

aliaspooryorik commented 12 years ago

Hi Greg - I've merged your pull request into the develop branch and also added the test. Thanks for the contribution!