Framework: [Proposal] Add Redirect::to()->withSuccess() - opposite to withErrors()

[DanSmith70]

Hi All,

I was wondering if there was any interest in adding the ability to use withSuccess when redirecting. Support for this would also need to be added to ViewServiceProvider.php to make the success messages global over all views. This would let you build in to your views a global (or per view) ability to respond to a success message much as we can do with error messages right now.

Some example use cases:

Redirect::home()->withSuccess('Thanks for registering, please check your email for an activation link');

Redirect::home()->withSuccess('Activation complete');

Redirect::home()->withSuccess('Your password reset email has been sent');

Redirect::home()->withSuccess('Your password has been updated');

Then perhaps in your master layout file you could add (blade/bootstrap example):

@if($success->any())
    <div class="alert alert-success alert-block">
        <button type="button" class="close" data-dismiss="alert">&times;</button>
        <h4>Success</h4>
        <ul>
            {{ implode('', $success->all('<li>:message</li>')) }}
        </ul>
    </div>
@endif

That would be optional of course, you could deal with it on a per view basis or not at all if you wanted.

Obviously this is possible with session flashing and then checking for the existence of the variable right now, but as we assured to always have an $errors MessageBag (empty or not) it makes sense to me to also have $success MessageBag too. This also reduces boilerplate on what must be a fairly common operation for a lot of users.

Thoughts?

Edit: I dont mind doing the PR for this if there is interest.

[crissi]

+1, or rename the $errors to $messages

[Anahkiasen]

Why not @if($errors->none()) ? I don't think it makes sense the way you present it, if there is any error you shouldn't have a success message.

[WMeldon]

@Anahkiasen One point against that is if you are using say Redirect::back(). $errors->none() will be true even if they haven't done anything.

In my mind "success" isn't just a lack of errors.

[Anahkiasen]

I think the problem here is trying to implement a notion of success on an $error variable. Whatever you do it won't semantically be correct I think. I'm more in favor of the $success variable mentionned in the first post and then  $success->any() or something.

[WMeldon]

Differentiating between success and errors in such a way could have some hidden complexity, but I still agree that it's a better solution.

I really don't think it's a good idea for successes to be found in errors.

[brunogaspar]

I think the example @Bulk70 have has a typo, it should be $success->any() and not $errors->any()

[WMeldon]

That makes a lot more sense

[DanSmith70]

Sorry yes @brunogaspar is right that was a copy/paste mistake, fixed now!

[JonoB]

Is it worthwhile just making this as flexible as possible?

->with('errors', 'This is an error message') // can be aliased to ->withErrors()
->with('success', 'It works!');
->with('foo', 'This is a foo message');

All this is really doing under the hood is using flash messages, so its pretty easy to do this manually anyway.

[robclancy]

Redirect::home()->with(['success' => 'message here']); is what I do and I think it is perfectly fine.

However I use have this stuff handled with a base class. If you do something often then think like a programmer and make it so you do it less often instead of requesting a specific method. Mine is $this->toAction('index', ['success' => 'message']);

[DanSmith70]

Its not quite just a flash message, it passes the errors though using a MessageBag which has built in formatting functionality amongst other benefits. My plan was to genericize the internal functions somewhat anyway as otherwise there would be a bunch of repeated code.

The other side of the problem is presenting the data to the views - which is the real point of this. The nice thing about $errors is you know its always there, and always going to be a MessageBag so you don't ever need to do an isset() on it, you can just use $errors->any() to check. To keep that same level of functionality with a generic message object is actually not so simple, because MessageBag's are not nested or associative in any way.

I do like the idea of passing a single generic messages variable to all views, which could then contain an array or object of some kind which could be parsed further. However I'd like to avoid the need have an isset on an array index or something like that. I need to have a look at the Collection object code and see if it could be used for associative data, but I think its probably more geared towards non-associative arrays of data right now - another proposal in it's self.

[robclancy]

Well what I basically do now, which is probably not the most "correct" way, is just flash it and do if ($success = Session::get('success'). Same thing for other alerts. Usually people have a helper.alerts view which will handle all these things.

I'm not against this idea I just don't know if it is something that should be in the core.

[helmut]

I like how the errors can be effortlessly passed back to the views. I agree that a positive message would be handy to have similar functionality to the negative (such as errors). Perhaps success is a bit too descriptive or rigid. Maybe we could call it feedback. Making the method withFeedback().

[DanSmith70]

@robclancy Yeah thats the only way right now, personally I just dont like that $errors is a nicley defined messagebag and we have to fend for our selves for the rest. Clearly it's not complicated code to handle it your self, it just feels "missing" to me.

To be honest I dont want to spend the time making and writing tests for this if @taylorotwell isn't onboard with it.

[spamoom]

+1 to this, would be nice to have the opposite shorthand

return \Redirect::to('/')
        ->with('successes', $bag);

return \Redirect::to('/')
        ->withSuccesses($bag);

[jurerick]

in my validation. i set the flush message

Session::flash('successMsg', 'Account successfully saved.');

in my BaseController, i set the success message as global to all my views, such like this

protected function setupLayout()
{

    View::share(array(
        'auth' => Auth::user(),
        'successMsg' =>  Session::get('successMsg'),
    ));

}

then, call the successMsg on a certain view

@if($successMsg)

{{$successMsg}}

@endif

[taylorotwell]

I don't think this is really necessary at the moment.

[vedmant]

There is withErrors() method, I think it should have full functionality, if you are not going to implement methods like withSucces(), withNotice(), withInfo(), withWarning(), withPopup(), it's better to remove withError() then. Make it consistent. IMHO.

[taylorotwell]

It makes absolutely no sense to remove a perfectly good method just because another method doesn’t exist.

On Sat, Oct 3, 2015 at 8:40 PM, Vedmant [email protected] wrote:

There is withErrors() method, I think it should have full functionality, if you are not going to implement methods like withSucces(), withNotice(), withInfo(), withWarning(), withPopup(), it's better to remove withError() then. Make it consistent. IMHO.

Reply to this email directly or view it on GitHub:
https://github.com/laravel/framework/issues/906#issuecomment-145305713

[vedmant]

Wow, didn't expect you to answer me! I'm just talking that having withErrors() makes a lot of people think that something like withSuccess() exists, but it doesn't. This is just how people think, they suppose if there is "A", then it have to be and "B" as well, but in this case there is only "A" and it makes filling that something is not complete here. If there were no method withErrors(), people wouldn't think that there may be something like withSuccess().

[lukasgeiter]

@vedmant you can actually use with* and the data will be flashed to the session with * as key. (code) So ->withSuccess() works, it's just not defined as an explicit method.

[ryvan-js]

Well whether laravel gonna implement the idea or not,i personally love to have the withSuccess().You can still customize the framework in such way to full-fill your expectation.

edit the following files:

[ 1 ] >> /vendor/laravel/framework/src/Illuminate/View/Middleware/ShareErrorsFromSession.php

add this line to the handle function just before the return syntax:

$this->view->share(
            'success', $request->session()->get('success', new ViewErrorBag)
);

so your code will look like this ;

  public function handle($request, Closure $next)
    {
        $this->view->share(
         'errors', $request->session()->get('errors', new ViewErrorBag)
        );
        $this->view->share(
         'success', $request->session()->get('messages', new ViewErrorBag)
        );
        return $next($request);
    } 

[ 2 ] >> /vendor/laravel/framework/src/Illuminate/Http/RedirectResponse.php

Add this function to the RedirectResponse class (before the closure) :

public function withSuccess($provider, $key = 'default')
    {
        $value = $this->parseErrors($provider);
        $this->session->flash(
            'success', $this->session->get('success', new ViewErrorBag)->put($key, $value)
        );

        return $this;
    }

[ 3 ] >> /vendor/laravel/framework/src/Illuminate/View/View.php

Add this function to the View class (before the closure) :

public function withSuccess($provider)
    {
        if ($provider instanceof MessageProvider) {
            $this->with('success', $provider->getMessageBag());
        } else {
            $this->with('success', new MessageBag((array) $provider));
        }

        return $this;
    }

The file [1] declared the custom shared variable "$success" and the [ 2 ] and [ 3 ] is to defining function which can push the messages to the view using the custom "$success" variable.

[mhayes14]

@ryvan-js It's not advised to edit files in the vendor directory because these will be overwritten at some point and probably won't be in version control.

I think the best solution currently is as @lukasgeiter mentioned.... withSuccess() magic method on the redirection, @if(session('success')) and {{ session('success') }} on the view.

[ryvan-js]

My solution is for those who willing to edit the core structure of their own development with confidence and who is pretty sure about what they are doing . I agree @lukasgeiter has a standard solution there..but not everyone likes to use the Session to catch the messages. I don't see why you should not edit your vendor directory if it can full-fill your requirement in a explicit way. But i agree your concern about version control will override them at some point,well you always can extend the native class files on top of yours so that the custom class files you are creating will be untouched by the version control.

[philbates35]

Editing the file in the vendor directory is a truely terrible idea, please don't do that, or suggest that others do that. Using withSuccess magic method is a much better solution.

[carkii]

+1 for the magic method, so we can save success, errors, info or warning messages :)

[lasithg]

seems to be serious problem without withSuccess(). i'm recently stuck with that and had to do more coding to catch the success message itself.

for example, using withErrors(), i can use same MessageBag to display error messages that using form validations. in this case i wanted to redirect previous page after password reset. so i had to use sessions instead of using same code blocks to display success message.

this is the code where i'm catching success messages;

           `@if (isset($errors) && count($errors) > 0)
                <div class="alert alert-danger">

                        @foreach ($errors->all() as $error)
                            {{ $error }} <br/>
                        @endforeach

                </div>
            @endif

        @if (isset($success) && count($success) > 0)
                <div class="alert alert-success">

                        @foreach ($success->all() as $succ)
                            {{ $succ }} <br/>
                        @endforeach

                </div>
            @endif

            @if(session()->has('success'))
                <div class="alert alert-success">
                    {{ session()->get('success') }}
                </div>
            @endif`

two code blocks for success message :(