October: Allow custom FormController redirects

[Flynsarmy]

There is currently no easy way to grab the saved model and redirect to a URL based on the data inside of it when saving a FormController form.

Use Case

I hit the 'Save & Preview' button on a blog post create page in admin. It should save the blog post, then redirect the browser to $model->permalink - a dynamically generated URL based on the slug assigned during the save process.

Proposal

Rename FormController.makeRedirect to FormController.formMakeRedirect and change all calls to $this->controller->formMakeRedirect. This change would be backwards compatible and allow overriding of the redirect behavior.

Alternatively, we could just add a POST variable to define the redirect URL override. Unfortunately 'redirect' is already being used as a simple boolean so we'd need to come up with another name that fits.

If you're happy with this proposal I can whip up a PR.

In Depth Issue

When FormController's create_onSave() method is called, the model is saved and then 3 calls are made:

$this->controller->formAfterSave($model);
$this->controller->formAfterCreate($model);
if ($redirect = $this->makeRedirect('create', $model)) {
    return $redirect;
}

• The first two lines contain my hydrated model but I can't return a Redirect::to($model->permalink) from them because it won't have any affect.
• Inside makeRedirect method there is no interaction with the controller and no POST value I can send to set the redirect URL.
• I can make the redirect happen from formAfterSave like so:

public function formAfterSave($model)
{
    if (post('preview')) {
        // Grab the current config
        $config = $this->asExtension('FormController')->getConfig();
        // Overwrite the redirect value with our new model's permalink
        $config->{$this->formGetContext()}['redirect'] =
            $model->permalink.'?preview=true';
        // Save the new config
        $this
            ->asExtension('FormController')
            ->setConfig($config);
    }
}

but obviously this isn't a great solution. It's messy and not very intuitive.

[LukeTowers]

@Flynsarmy I would find it acceptable to make the getRedirectUrl() method overridable and include $model in the parameters. So $this->controller->formGetRedirectUrl($context, $model = null)

[Flynsarmy]

I'm happy with this solution and will whip up a PR.

If we look at the current makeRedirect method:
https://github.com/octobercms/october/blob/d46accfc4f309f5f071cee41ac5bf58092559392/modules/backend/behaviors/FormController.php#L462-L490

How's this look:

     public function makeRedirect($context = null, $model = null) 
     { 
        if ($redirectUrl = $this->controller->formGetRedirectUrl($context, $model) {
            return Redirect::to($redirectUrl);
        }

        ...
     }

I don't think we can just set $redirectUrl to formGetRedirectURL() if one is returned and continue down the makeRedirect method as normal, because it's not intuitive that a relative URL returned from this method will lead to backend and absolute to frontend. Thoughts?

[LukeTowers]

@Flynsarmy I think it's fine to keep the existing behavior of relative vs absolute. This method will need to be documented anyways so we can include that tidbit in a note. I think it is intuitive to some extent in that the existing redirectUrl usage is relative and leads you to the backend. A simple note in the docs added should be sufficient I would think. Could be included in the docblock too in case someone finds the method by reading the code instead of the docs.

[Flynsarmy]

Thanks. I'll whip up a PR shortly with relevant docblock notes.

[LukeTowers]

@Flynsarmy sounds good. Please submit a PR to the docs repo as well.

[github-actions[bot]]

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days.
If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue.
If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[Flynsarmy]

Ping