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

WIP update - validation only provided elements #1560

Closed
wants to merge 2 commits into from

Conversation

nowackipawel
Copy link
Contributor

@nowackipawel nowackipawel commented Nov 30, 2018

According to #1559 I write some code which allows to filter validationRules to that elements which are provided in $data.


Allows to filter validation rules to only that elements which are in $data passed to update method
If you'd like to check only fields passed in $data during update just call:
$this->update($your_id, $your_data, true);

if there is 5 rules in your $validation_rules and only two of them in $your_data array then only that two elements will be checked in validation process if you pass true as a 3rd arg (if you don't provide it or set to false then update will check every rule as is now).

Useful: i.e. you want to only update user nickname or password instead of passing all user attributes.

Each pull request should address a single issue, and have a meaningful title.

According to codeigniter4#1559 I write some code which allows to filter validationRules to that elements which are provided in $data.

----

Allows to filter validation rules to only that elements which are in $data passed to update method
If you'd like to check only fields passed in $data during update just call:
$this->update($your_id, $your_data, true);


if there is 5 rules in your $validation_rules and  only two of them in $your_data array then only that two elements will be checked in validation process if you pass true as a 3rd arg (if you don't provide it or set to false then update will check every rule as is now). 

Useful: i.e. you want to only update user nickname or password instead of passing all user attributes.
@nowackipawel
Copy link
Contributor Author

Oh, I've already see... in my commit there is also a code from my PR#1325 cuz I am using it more than a month. I think you can close PR#1325 and if you decide too please merge this one and only docs from PR1325.

@jim-parry
Copy link
Contributor

If the docs from 1325 belong with this, they should be part of this PR too.
Does the same apply to the unit testing from 1325?

@jim-parry jim-parry changed the title update - validation only provided elements WIP update - validation only provided elements Dec 9, 2018
@nowackipawel
Copy link
Contributor Author

nowackipawel commented Dec 18, 2018

Nope. In pr#1325 add support for additional options passed in the 1st parameter to update() method. This PR ads support for running validation only for fields which are provided in 2nd parameter if 3rd is set to true.
** update: #1325 - replaced by #1644 **

@nowackipawel
Copy link
Contributor Author

@lonnieezell , @jim-parry
do you want me to do anything else with this?
or should I wait for merging other Model's stuff (i.e. #1644) and then make new PR once again?
/I am asking you because in this PR I worked on my copy of Model.php and there is some stuff from #1325 which I asked you to close./

@jim-parry
Copy link
Contributor

Missing user guide update? unit tests?
You said your PRs are getting a bit confusing with "cross-pollination", i.e. using your already-updated Model in them. Each should stand on its own, and hopefully the mixups will "settle down" once we have worked through the current changes :)

@nowackipawel
Copy link
Contributor Author

I'd like to ask someone for help with unit tests. Have we got someone who will be able to write tests?
I am after my deadlines and I don't want to make things worse :(. I won't be able to write it in (at least) the next few days/weeks - OFC if there will be anything unclear for that person I will help ASAP as well in case of any issues/bugs with this code ( I've tested it during developing an webapp, but you know...).

I could make new PR ASAP with this when #1644 will be merged because probably it will be conflicted with #1644 .

@jim-parry
Copy link
Contributor

Unit test help would be me, and not for the next week too :-/
Let's wait until 1644 is done, to minimize conflicts.

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

Successfully merging this pull request may close these issues.

3 participants