barrelstrength/sprout-fields

Upgrade from v3.4.4 to v3.6.2 loses data on the address table

Closed this issue · 5 comments

brnquester commented

Description

We have upgraded the package from 3.4.4 to 3.6.2. Before the upgrade, our table craft_sproutfields_addresses had 932 records, after the upgrade 47 in the new table craft_sprout_addresses.

Any idea what is the reason?

Additional info

  • Craft version: 3.3 > 3.4.14 (We did the upgrade together with this package upgrade)
  • PHP version: 7.2
  • Database driver & version: Mysql v14.14
  • Plugins & versions:

Front logo Front conversations

BenParizek commented

@bruno-canada There were several updates to addresses between these two releases. We did have a scenario where some draft entries were orphaned in the address table after the Drafts were deleted.

It's not clear from your question but can you confirm if you are seeing all the addresses you expect to see in the CP or if the discrepancy is indicative that several addresses you expect to be able to see have been deleted?

For reference, the most significant migration was this one m200102_000000_remove_address_field_content_column_sproutfields

And the new deleteUnusedAddresses method uses this logic:

public function deleteUnusedAddresses()
    {
        $addressIdsWithDeletedElementIds = (new Query())
            ->select('addresses.id')
            ->from('{{%sproutfields_addresses}} addresses')
            ->leftJoin('{{%elements}} elements', '[[addresses.elementId]] = [[elements.id]]')
            ->where(['elements.id' => null])
            ->column();

            Craft::$app->db->createCommand()
            ->delete('{{%sproutfields_addresses}}', [
                'id' => $addressIdsWithDeletedElementIds
            ])
            ->execute();
    }

Please let me know if you feel any content has been incorrectly deleted and we can dig in more. If so, can you please send a copy of your db over to sprout@barrelstrengthdesign.com to help troubleshoot?

brnquester commented

@BenParizek Thank you for taking the time to reply. Your information is very valuable.

Here are my finding.

  1. I have commented the code inside deleteUnusedAddresses(), restored my database and then run migrate again. The records were gone anyways.

  2. I have enabled MYSQL general_log to watch all queries going on when adding an address, during migration and so on. After some time checking the queries I have written a query that connects all possible relationships based on what I have watched on MYSQL general logs with the purpose of finding where my records are missing the link.
    Tables that I was able to discover:

    • craft_content

    • craft_elements

    • craft_elements_sites

    • craft_matrixcontent_location

    • craft_matrixcontent_listinglocation

    • craft_matrixblocks

    • craft_supertableblocks

    • Query to check relationship record numbers:
      SELECT sproutaddr.id as sprout_addresses_id,(SELECT COUNT(*) FROM craft_content WHERE elementId = sproutaddr.elementId) as craft_content,(SELECT COUNT(*) FROM craft_elements WHERE id = sproutaddr.elementId) as craft_elements,(SELECT COUNT(*) FROM craft_elements_sites WHERE elementId = sproutaddr.elementId) as craft_elements_sites,(SELECT COUNT(*) FROM craft_matrixcontent_location WHERE field_placeData_streetAddress = sproutaddr.id) as craft_matrixcontent_location,(SELECT COUNT(*) FROM craft_matrixcontent_listinglocation WHERE field_location_streetAddress = sproutaddr.elementId) as craft_matrixcontent_listinglocation,(SELECT COUNT(*) FROM craft_matrixblocks WHERE id = sproutaddr.elementId) as craft_matrixblocks, (SELECT COUNT(*) FROM craft_supertableblocks WHERE ownerId = sproutaddr.elementId) as craft_supertableblocks FROM craft_sprout_addresses as sproutaddr

    • To my surprise, all addresses missing after the migration are the ones without a link to the craft_content table. I was able to match the number running the following query:
      SELECT COUNT(*) FROM craft_sprout_addresses as sproutaddr WHERE EXISTS(SELECT * FROM craft_content WHERE craft_content.elementId = sproutaddr.elementId)

    • Most of the other records in the table craft_sproutfields_addresses that were lost during migration are linked to craft_matrixcontent_location and craft_matrixcontent_listinglocation

  3. In your migration file when transferring the records from the old table to the new one you only do that based on craft_contents and not the matrixcontent. Right?
    m200102_000000_remove_address_field_content_column.php#L65

    • I know you are aware of it but just as a heads up the fields name will have to be changed as well to follow the logic applied to matrixcontent field_MATRIXNAME_fieldname.
      m200102_000000_remove_address_field_content_column.php#L52
    • I have also noticed that we had a column called field_streetAddress that was deleted after migration;

  4. The migration logic fails for multiple fields with the same handle since it drops it on the first loop for each field name, in our case, it is part of the issue:

    • Table: craft_fields - results for your migration search:
      Id: 3
      handle: streetAddress
      context: global
      type: barrelstrength\sproutfields\fields\Address
    • Id: 87
      handle: streetAddress
      context: matrixBlockType:f68c5293-0b48-4fdc-9431-4d4bb182e8ea
      type: barrelstrength\sproutfields\fields\Address
    • Id: 196
      handle: streetAddress
      context: matrixBlockType:bf728e4a-dbbb-4569-9ba1-16923d4ea5af
      type: barrelstrength\sproutfields\fields\Address

m200102_000000_remove_address_field_content_column.php#L90

BenParizek commented

@bruno-canada My apologies for the delay here. I just release a blocking update that I was working on and will be taking a closer look at this shortly. I believe you've identified the issue and we'll need to update that migration to address matrix blocks as well.

brnquester commented

@BenParizek It has been 14 days since we heard from you. This is creating unnecessary stress with the client. Please, advise.

BenParizek commented

@bruno-canada I've just released Sprout Fields v3.8.1 which should address the bug in this migration. You'll need to re-run the update as the fix had to happen in the original migration.

Thanks for your detailed report. It helped identify the issue and figure out what was going on. This latest release adds support for both Matrix Fields and SuperTable fields.

This will also address the logic with multiple fields using the same handle. The code here is not dropping the field itself but the old table column for each field as each of these use field contexts (global, matrixBlock, superTableBlock) all use different content tables (previously, some address data was stored in the content table column but that is no longer necessary).

  • 👍1