Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialized data not compatible with MCW #35

Open
fritzmg opened this issue Feb 24, 2022 · 3 comments
Open

Serialized data not compatible with MCW #35

fritzmg opened this issue Feb 24, 2022 · 3 comments
Labels
feature New feature or request

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Feb 24, 2022

I wanted to start replacing MCW with the group widget in my extensions and I was hoping that the serialized data would be compatible, so that no migration is necessary. Unfortunately there is one tiny difference which breaks the compatibility. This is what a serialized array from MCW looks like for instance:

a:1:{i:0;a:2:{s:6:"format";s:4:"json";s:8:"template";s:28:"nc_fieldset_duplication_json";}}

And this from the group widget:

a:1:{i:1;a:2:{s:6:"format";s:4:"json";s:8:"template";s:28:"nc_fieldset_duplication_json";}}

The key difference is the array index. The array produced by MCW starts with 0 whereas the array produced by the group widget starts at 1 for some reason. The group widget is then not able to load the already existing data because of that.

Is there may be an easy fix without also breaking BC? 🤔 Would be convenient to be able to replace the MCW directly with the group widget, without any migration :)

@m-vo
Copy link
Owner

m-vo commented Feb 24, 2022

This would be interesting indeed.

I remember there was a problem with using 0-based IDs and that's why they are treated as invalid in normalizeGroupData() and stepped over in createElement(), but I cannot recall the exact problem anymore. But if we can detect these states, we could probably automatically migrate them. 🤔

@m-vo m-vo added the feature New feature or request label Feb 24, 2022
@fritzmg
Copy link
Contributor Author

fritzmg commented Feb 24, 2022

I ended up writing a simple migration anyway, but it would be cool to be able to replace MCW directly, yes :)

class NotificationTokenTemplatesMigration extends AbstractMigration
{
    private $db;

    public function __construct(Connection $db)
    {
        $this->db = $db;
    }

    public function shouldRun(): bool
    {
        $schemaManager = $this->db->getSchemaManager();

        if (!$schemaManager->tablesExist(['tl_form_field'])) {
            return false;
        }

        $columns = $schemaManager->listTableColumns('tl_form_field');

        if (!isset($columns['notificationtokentemplates'])) {
            return false;
        }

        return (int) $this->db->fetchOne("SELECT COUNT(*) FROM tl_form_field WHERE notificationTokenTemplates LIKE 'a:1:{i:0;%'") > 0;
    }

    public function run(): MigrationResult
    {
        foreach ($this->db->fetchAllAssociative("SELECT * FROM tl_form_field WHERE notificationTokenTemplates LIKE 'a:1:{i:0;%'") as $field) {
            $templates = [];

            foreach (StringUtil::deserialize($field['notificationTokenTemplates'], true) as $key => $template) {
                if (is_numeric($key)) {
                    $key = $key + 1;
                }

                $templates[$key] = $template;
            }

            $this->db->update('tl_form_field', ['notificationTokenTemplates' => serialize($templates)], ['id' => $field['id']]);
        }

        return $this->createResult(true);
    }
}

@m-vo
Copy link
Owner

m-vo commented Feb 25, 2022

I think we could maybe switch to zero-based indices in the future. I tested some quick changes in #38 and they seem to work. Not sure if this affects other storages as well as we're also not filtering 0-values anymore in the listener now. But likely that this was only a leftover from earlier refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants