Fixed deleteWidget in AbstractSetup does not appear to delete the MasterTitle phrase

Affected version
2.0.2

DragonByte Tech

Well-known member
#1
Calling $this->deleteWidget($widgetKey); in the uninstaller leaves orphan phrases:

1519516350605.png

EDIT: Just to confirm, it does actually delete the widget correctly, just not the phrase.


Fillip
 

DragonByte Tech

Well-known member
#3
I've just tested this with the add-on below and every time the widget is deleted.
Can you please test it with the eCommerce mod I attached in the conversation? It's the only mod I investigated this with, but my DB is full of imported data and it takes a long time to re-populate :D

EDIT: Also, just to be sure, the widget always deletes successfully, but the phrases shown in the screenshot did not (for me).


Fillip
 

DragonByte Tech

Well-known member
#5
Can repro it with your add-on now :rolleyes:
See, I'm not crazy!

If you notice in the Setup.php file, I split a lot of phrase deletions into separate installer steps. I received multiple execution timeout errors if I tried to delete all orphan content phrases in one go due to the DevOutput file deletion, so maybe it's related to that?


Fillip
 

Chris D

XenForo developer
Staff member
#6
I don't think that's related, no. I've been doing the installs/uninstalls of your add-on using the CLI so that shouldn't be susceptible to any timeouts.

I notice that one of the widgets does have its phrase deleted and that's the member stat one - so the only one that uses a default widget definition (which isn't deleted during uninstall of course) works as expected.

I sort of pre-empted that could be a factor but that's why I tested that scenario in the test add-on above.

Either way, the Widget Definition entity deletes any widget entities which use that definition, and the deleteWidget method uses the entity to delete so either way I look at it, the master phrase should get deleted.

:unsure:
 

DragonByte Tech

Well-known member
#7
Either way, the Widget Definition entity deletes any widget entities which use that definition, and the deleteWidget method uses the entity to delete so either way I look at it, the master phrase should get deleted.
The plot, like me when I look at a piece of bread, thickens.

It appears as if the MasterTitle phrase entity is flagged as deleted:

PHP:
        if ($this->_deleted)
        {
            die('already deleted');
            return true;
        }
Will continue to dig deeper.


Fillip
 

DragonByte Tech

Well-known member
#8
@Chris D Upon further inspection, this was actually due to a bug in the Setup.php file, but the subsequent cause is actually this block of code:

PHP:
    public function createWidget($widgetKey, $definitionId, array $config, $title = '')
    {
        /** @var \XF\Entity\Widget $widget */
        $widget = $this->app->em()->create('XF:Widget');
        $widget->widget_key = $widgetKey;
        $widget->definition_id = $definitionId;
        $widget->bulkSet($config);
        $widget->save(false);

        $masterTitle = $widget->getMasterPhrase();
        $masterTitle->phrase_text = $title;
        $masterTitle->save(false);
    }
If the widget fails to insert for whatever reason, the phrase is still created.

Might I suggest the following:

PHP:
    public function createWidget($widgetKey, $definitionId, array $config, $title = '')
    {
        /** @var \XF\Entity\Widget $widget */
        $widget = $this->app->em()->create('XF:Widget');
        $widget->widget_key = $widgetKey;
        $widget->definition_id = $definitionId;
        $widget->bulkSet($config);
        $success = $widget->save(false);
        
        if ($success)
        {
            $masterTitle = $widget->getMasterPhrase();
            $masterTitle->phrase_text = $title;
            $masterTitle->save(false);
        }
    }
That would take care of the issue where an incorrect widget configuration would still generate a phrase that can never be removed :D


Fillip
 
Top