XF 2.2 forceSetDiscussionType() not working in some cases?

[Jaxel]

I am extending the thread entity with the following class:

    public function canView(&$error = null)
    {
        if (***true***)
        {
            $this->forceSetDiscussionType('article');
        }
       
        return parent::canView($error);
    }

Pretty simple. If the statement is true, force the discussion view to an "article". This is working fine.

However, when I am using the inline-editor on the first post of said article thread, when I click save, instead of returning the post in the article format, it returns it as a general discussion thread format, with the user signature instead of the author info block. Is there a reason why this function is not firing during an inline-edit?

 

[Jaxel]

I figured it out. The issue is related to the service thread editor validation.

Even if you force the discussion type; when the thread editor is validated, it reverts the forced type to the actual type.

I have solved this issue by adding the following function:

    protected function _postSave()
    {
        if (***true***)
        {
            $this->forceSetDiscussionType('article');
        }
        
        return parent::_postSave();
    }

 

[Mike]

I'm honestly confused about what you're trying to achieve here, but your approach seems quite dangerous and could theoretically lead to data loss in some situations. The fact that you can't set the discussion type to article because the forum doesn't allow article threads and that we don't allow this to be saves should be an indication that you should probably seek an alternative approach.

If you can explain what you're trying to do, we might be able to give some alternative ideas.

 

[Jaxel]

How would this result in data loss? All it's doing is telling the internal templater to load the article templates, instead of the default discussion templates.

 

[Mike]

No, you're actually changing the value of the discussion_type field such that if a save happens, the thread will actively be converted to the article type. This leads to various lifecycle methods being called because the thread type is potentially being changed -- these might clean up data that isn't applicable any longer.

Beyond that, as you noticed, preSave validates that the type is actually valid. If it's not, it will force the type to be valid in the forum, and this might not be the type the thread was before, meaning that it's still triggering the lifecycle behaviors (and very specifically will wipe out any type_data values that might've been stored for that thread).

 

[Jaxel]

No, you're actually changing the value of the discussion_type field such that if a save happens, the thread will actively be converted to the article type. This leads to various lifecycle methods being called because the thread type is potentially being changed -- these might clean up data that isn't applicable any longer.

Beyond that, as you noticed, preSave validates that the type is actually valid. If it's not, it will force the type to be valid in the forum, and this might not be the type the thread was before, meaning that it's still triggering the lifecycle behaviors (and very specifically will wipe out any type_data values that might've been stored for that thread).

Looking at the forceSetDiscussionType function itself, that appears not to be the case if I only send the first parameter. It's the second parameter which will change the thread type... which I am not using. Or am I misunderstanding how that works?

    public function forceSetDiscussionType($type, array $typeData = null)
    {
        $this->_setInternal('discussion_type', $type);
        if ($typeData !== null)
        {
            $this->type_data = $typeData;
        }
    }

 

[Jaxel]

I just did a test, and executed the following code:

        $thread->forceSetDiscussionType('article');
        $thread->save();

It did not replace the discussion_type in the database.

And I would think the fact that forceSetDiscussionType doesn't change the discussion_type in the database is why I was having issue reported in the OP in the first place. Because the lifecycle of saving on validation, reverted the discussion_type into it's actual value instead, of "article".

 

[Jaxel]

But if I am in fact, misunderstanding how _setInternal works...

What would be the best way to force a thread to use the "article" TypeHandler, instead of the "discussion" type handler?

 

[Jaxel]

I'm so dumb. I can just do this can't I?

    public function getTypeHandler(bool $current = true, bool $fallback = true)
    {
        if (***true***)
        {
            return $this->app()->threadType('article', false);
        }
       
        return parent::getTypeHandler($current, $fallback);
    }

 

[Jaxel]

Yep. It worked. Solved the lifecycle issue... and the race condition issue in my OP.

Thanks @Mike !

 

[Mike]

That does seem like a better approach, though it might still create some oddities as you could be creating inconsistencies in some scenarios (as the type handler returned won't be that correct one for the expected type). I'm still not really clear on why you need to force the article type handler instead of either changing the type to article or creating your own handler that can function as you want.

In terms of _setInternal, consider it doing the same thing as set except it skips all of the validation and coercion stuff. It assumes what you're giving it is totally valid. So if you ran that example code on a poll thread (in a forum that didn't allow articles), you'd probably end up with a discussion thread and the poll would be lost.

On a side note, forceSetDiscussionType has actually just been removed. It was more of a holdover from something that was needed in an earlier variaion of the thread types system. It isn't used internally and the use case that necessitated it doesn't really exist any more, so best to remove it from the core implementation while we can (pre-2.2 stable).

 

[Jaxel]

In XenPorta, it allows you to promote ANY thread, from any forum to the portal. Forcing the type to "article", allows it to display any thread as an article, even if it doesn't come from an article forum.

 

[Jaxel]

I see now why you told me not to do it this way. It doesn't play nice with the new template extension system.