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

Update to real_save's "sidestep of validation" #110

Closed
wants to merge 1 commit into from
Closed

Update to real_save's "sidestep of validation" #110

wants to merge 1 commit into from

Conversation

nightowl77
Copy link

Hi Guys

I had a look at andrew13's postEdit in the great Laravel-4-Bootstrap-Starter site (https://github.com/andrew13/Laravel-4-Bootstrap-Starter-Site/blob/master/app/controllers/admin/AdminUsersController.php#L169)

The postEdit seemed to have way too much logic for handling the password and password confirmation.

I simplified the code to

 public function postEdit($user)
    {

            $user->username = Input::get('username');
            $user->email = Input::get('email');

            if (Input::get('password'))
            {
                $user->password = Input::get('password');
                $user->password_confirmation = Input::get('password_confirmation');
            }

            $user->confirmed = Input::get('confirm');

            $user->amend();

        $error = $user->errors()->all();

        if(empty($error)) {
            // Redirect to the new user page
            return Redirect::to('edt/users/' . $user->id . '/edit')->with('success', Lang::get('admin/users/messages.edit.success'));
        } else {
                return Redirect::to('edt/users/' . $user->id . '/edit')->withErrors($error);
        }
    }

but this didn't quite work.

On close inspection I found an if statement checking if $rules are empty in line 275 of ConfideUser.php. Basically $rules are passed all the time meaning this IF would never fire.

In this pull request I took the other route, instead of making a new set of rules, I simply delete the "password" rule from a pre-exiting set of rules.

With this change the code above runs correctly, ie. when the user enters a password, it is changes, but if he leaves it empty, it will stay the same without a validation error.

I also ran the PHPunit tests for Confide, all OK.

@nightowl77
Copy link
Author

Any feedback on this? The comment just above the if statement (line 272) says:

This will make sure that a non modified password
will not trigger validation error.

The changes I made ensure that the code reflects what the comment says. It also nicely abstracts the management of the password validation away from the developer, and every developer using Confide don't have to code all that "if (empty($pasword)) ..... unset($user->password)"

@nightowl77 nightowl77 closed this Jul 19, 2013
@andrew13
Copy link
Collaborator

The downside to this is it hides the rule, instead of having all the rules in a class variable.

@andrew13
Copy link
Collaborator

Looks good. Adding this shortly.

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.

None yet

2 participants