XF 2.0 Get variables from parent class, on a $redirect?

[Jaxel]

I'm trying to extend a class. But the class doesn't return a $view, it returns a $redirect.

Normally, with a $view, I could use getParams() to get the view parameters.

Naturally, I can't do this on a redirect, since there are no view parameters. Is there an easy way I can access the parent parameters?

What I'm trying extend is XF\Forum:actionPostThread, on thread creation post. I need to know the $thread it just created.

 

[DragonByte Tech]

Looking at the controller in question, I think you might have better luck if you extend finalizeThreadCreate instead. I can't see anything else in the controller below the call to that function that should apply to your specific use case.

My initial impressions of how that would go:

    protected function finalizeThreadCreate(\XF\Service\Thread\Creator $creator)
    {
        parent::finalizeThreadCreate($creator);

        $forum = $creator->getForum();
        $thread = $creator->getThread();
        $visitor = \XF::visitor();
        
        // Inspect contents or obtain POST params via $this->filter here
    }

Fillip

 

[Jaxel]

Yeah... finalizeThreadCreate() doesn't return anything though...

What I wanted to do, was on specific settings, return the viewer a redirect to a different page. But since finalizeThreadCreate() doesn't have it's own return, I can't send the user anywhere. If I put a return in there, it justs gets lost in the aether.

 

[DragonByte Tech]

Yeah... finalizeThreadCreate() doesn't return anything though...

What I wanted to do, was on specific settings, return the viewer a redirect to a different page. But since finalizeThreadCreate() doesn't have it's own return, I can't send the user anywhere. If I put a return in there, it justs gets lost in the aether.

throw $this->exception($this->redirect($this->buildLink())); (found in XF\Install\Controller\Upgrade) - try that


Fillip

 

[Jaxel]

throw $this->exception($this->redirect($this->buildLink())); (found in XF\Install\Controller\Upgrade) - try that


Fillip

That works... seems kind of wrong though.

 

[DragonByte Tech]

That works... seems kind of wrong though.

If it was wrong, would XF themselves use it in the upgrade tool?

I'm sure @Chris D or @Mike will chime in if I've committed some form of sin that they only use in the installer because they didn't expect some cabbage to go digging through all the files looking for a solution to this


Fillip

 

[Chris D]

That approach is reasonable.

 

[Snog]

This might be a reasonable approach, but IMHO it's poor practice.

Any other add-on that expects it's results to be redirected to it's own page and/or needs to do additional processing after finalizeThreadCreate runs will never get there because of redirecting from finalizeThreadCreate.

The only way around it is for everyone to create their own finalizeThreadCreate function and use it. Or, account for the redirect in other ways.

 

[PatM]

Can't you use a finder to get the latest thread in the node started by the user?

 

[Robust]

This might be a reasonable approach, but IMHO it's poor practice.

Any other add-on that expects it's results to be redirected to it's own page and/or needs to do additional processing after finalizeThreadCreate runs will never get there because of redirecting from finalizeThreadCreate.

The only way around it is for everyone to create their own finalizeThreadCreate function and use it. Or, account for the redirect in other ways.

I suppose you could fiddle around with the extension order to try and ensure it's fired last, to allow all normal events to take place first.

 

[Snog]

I suppose you could fiddle around with the extension order to try and ensure it's fired last, to allow all normal events to take place first.

Since that function doesn't normally return anything, it just executes, the extension order makes no difference. If it's used to redirect to a url by any extension, once called, anything after the call in the calling function will not be executed.

 

[Robust]

Since that function doesn't normally return anything, it just executes, the extension order makes no difference. If it's used to redirect to a url by any extension, once called, anything after the call in the calling function will not be executed.

I'm aware of what you're syaing. If it executes last, the code before it has already ran, there should not be anything after the redirecting function, so it should have no effect on normal operation.

 

[Snog]

I'm aware of what you're syaing. If it executes last, the code before it has already ran, there should not be anything after the redirecting function, so it should have no effect on normal operation.

So you're saying my own add-on should not be able to return to it's own page?

I don't think you understand so I'll do some fake code to show what I mean...

An extension of actionPostThread:

public function actionPostThread(ParameterBag $params)
{
    if ($this->isPost())
    {
        .... construct post, and create creator

        $thread = $creator->save();
        $this->finalizeThreadCreate($creator);
       
        CODE MUST RETURN TO MY PAGE
        BUT CAN NOT GET HERE WITHOUT OVER RIDING finalizeThreadCreate
        OR EXECUTING MY OWN finalizeThreadCreate EXTENSION BEFORE SOMEONE ELSE'S
        AND IF I DO THAT, THE REDIRECT FROM MY EXTENSION WILL MESS UP OTHER ADD-ONS
    }
}

I've gotten around the problem. My point is, it's not very friendly and normally would not be expected to redirect from finalizeThreadCreate.

 

[DragonByte Tech]

So you're saying my own add-on should not be able to return to it's own page?

I don't think you understand so I'll do some fake code to show what I mean...

An extension of actionPostThread:

public function actionPostThread(ParameterBag $params)
{
    if ($this->isPost())
    {
        .... construct post, and create creator

        $thread = $creator->save();
        $this->finalizeThreadCreate($creator);
     
        CODE MUST RETURN TO MY PAGE
        BUT CAN NOT GET HERE WITHOUT OVER RIDING finalizeThreadCreate
        OR EXECUTING MY OWN finalizeThreadCreate EXTENSION BEFORE SOMEONE ELSE'S
        AND IF I DO THAT, THE REDIRECT FROM MY EXTENSION WILL MESS UP OTHER ADD-ONS
    }
}

I've gotten around the problem. My point is, it's not very friendly and normally would not be expected to redirect from finalizeThreadCreate.

To be honest, I'm not sure what you're getting at either.

The point I think Robust was trying to make is the fact that if you need to run code inside finalizeThreadCreate, you do that with the normal execution order (or a value below 65535, or whatever the maximum is in XF2). Therefore, your code runs before Jaxel's redirect.

What I think you are trying to say is that if both you and Jaxel need to redirect after posting a thread, only one redirect will occur. If that is indeed what you are saying, then I don't see this as an issue because (presumably) both you and Jaxel will be limiting the redirect code to only run if the thread was created in special circumstances (such as a special URL parameter that would only exist if the thread was being created from inside your mod).

If I am correct in my assumption, then the only way this could possibly conflict, as far as I can tell, is if you add your special URL parameter to every thread form, so that both your param and Jaxel's param appear on the form at the same time.

If I am not correct, then in my opinion your fake code is 100% wrong because you're not extending actionPostThread, you're replacing it, which would always break compatibility with other modifications anyway, redirect or not.

If I am still missing the point, could you please elaborate?

EDIT: Let's also not forget that the scope of this conflict is very limited - new threads created automatically uses the Thread Creator service, not the form, so that is irrelevant here. Maybe I'm missing something obvious but I'm struggling to see where the major problem is.


Fillip

 

[Snog]

You're correct in the case I gave it would be replacing the thread post routine. And I do that in several specialized situations where the thread is built from data obtained from an external source.

But my point is that if everyone redirects out of finalizeThreadCreate (even with qualifiers), eventually everyone's add-on would be broken as a result of people executing their own code before other add-ons.

In a simplified extended example...

Add-on Z extends finalizeThreadCreate and executes before Add-on Y <-- THIS IS THE ONLY ONE THAT WOULD RUN
Add-on Y extends finalizeThreadCreate and executes before Add-on X (never runs)
Add-on X extends finalizeThreadCreate and executes before mine (never runs)
I extend finalizeThreadCreate and execute before Jaxels' (never runs)
Jaxel's never runs

This was not my solution, however it can be a never ending situation of finalizeThreadCreate returning to the wrong page.

As I said, I have solved the problem. I just don't think it's very friendly.

EDIT: I don't know how else to explain it better. And I suppose the same situation is theoretically possible with any extended function. So my point may be moot.

 

[DragonByte Tech]

You're correct in the case I gave it would be replacing the thread post routine. And I do that in several specialized situations where the thread is built from data obtained from an external source.

If you've already accepted that your needs are so specific the regular actionPostThread won't suffice, wouldn't it make more sense to create a whole new controller and routine? I think your solution of replacing the whole routine is not very friendly either, if you'll permit me to be blunt

But my point is that if everyone redirects out of finalizeThreadCreate (even with qualifiers), eventually everyone's add-on would be broken as a result of people executing their own code before other add-ons.

Yes, that's true, but as I said, I don't believe this will be a widespread issue. As long as every developer is careful to always set their execution order to the maximum, and qualify their redirects properly based on form input, then it's not going to be a problem IMO.

There is literally no solution that I can see short of creating a new controller, so it's my belief that as long as the redirects are properly qualified, there's no better solution.


Fillip

 

[Snog]

If you've already accepted that your needs are so specific the regular actionPostThread won't suffice, wouldn't it make more sense to create a whole new controller and routine? I think your solution of replacing the whole routine is not very friendly either, if you'll permit me to be blunt

No problem being blunt.

The replacement code is usually qualified by at least 2 different checks. If those checks aren't met, the parent is used and no harm, no foul.

I would still have to check what is happening in the thread creation process to divert to a new controller anyway. Or, completely change the "Post new thread" link to the new controller which would render other add-ons inoperable too.

So, it's a choice of the lesser of two evils.

 

[Snog]

As long as every developer is careful to always set their execution order to the maximum, and qualify their redirects properly based on form input, then it's not going to be a problem IMO.

No matter if every developer qualifies their redirect, someone's will not get executed. This is where this comes in...

Add-on Z extends finalizeThreadCreate with a redirect
Add-on Y extends finalizeThreadCreate with a redirect
Add-on X extends finalizeThreadCreate with a redirect
I extend finalizeThreadCreate with a redirect
Jaxel's with a redirect

Only ONE of those will redirect where it's expected to redirect to.

But, again. I've solved the problem. And it is a fringe problem that shouldn't happen often.

 

[Robust]

No matter if every developer qualifies their redirect, someone's will not get executed. This is where this comes in...

Add-on Z extends finalizeThreadCreate with a redirect
Add-on Y extends finalizeThreadCreate with a redirect
Add-on X extends finalizeThreadCreate with a redirect
I extend finalizeThreadCreate with a redirect
Jaxel's with a redirect

Only ONE of those will redirect where it's expected to redirect to.

But, again. I've solved the problem. And it is a fringe problem that shouldn't happen often.

My point was that normal code will be executed simply by setting execution orders (ensure the redirect execution listener runs last). As for other redirects, that's not the big issue (it would be a bigger issue if another add-on ceased to work properly due to this method, which is solved by doing what I mentioned).

Jaxel is doing conditional redirects. Your conditions will not match his conditions, hence there will be no redirect conflict. If any of the add-ons is doing redirects and there is a conflict in conditions, sure, only one redirect will work but this isn't a big deal, and it's an edge case. The other issues outlined with the method described earlier in this thread can all be mitigated.

DB Tech's quote sums up what I was trying to say:

As long as every developer is careful to always set their execution order to the maximum, and qualify their redirects properly based on form input, then it's not going to be a problem IMO.

 

[Snog]

I guess nobody can see the original point of the stock XF function not redirecting anywhere and it not being expected to redirect anywhere. And if it's not expected, then it CAN cause issues for people. I'm not talking about any specific case as it has drifted into.

So based on that, I'll go quietly into the night.