nerds-and-company / schematic

Craft Setup Sync
MIT License
136 stars 15 forks source link

Assets export/import bug with volumes and volumefolders (Craft 3) #156

Closed thijskaspers closed 5 years ago

thijskaspers commented 6 years ago

Hiya,

Found a bug with exporting/importing assets. When you create an asset field, one of the attributes is the 'folder' setting, for example 'folder:3'. The ID of that folder references a row in the 'volumefolders' table and the volume folder has a volume ID pointing to the volume table.

Lets assume I have 2 volumes:

volumes table

ID | Name | Handle
------------------
1  | Home | home
2  | Blog | blog

I now create 3 asset fields; one will be stored in 'home', one will be stored in 'blog', and the last one will be stored in 'home' with subfolder 'header'. Craft now creates an extra volumefolder row, where the new one is volume home with folder 'header':

volumefolders table

ID | Volume ID | Name        | Path
-------------------------------------
1  | 1         | Home        | 
2  | 2         | Blog        |
3  | 1         | home_header | header

The issue

When you now run a schematic export, schematic will throw a warning saying 'No mapping found for source folder:3' and an import will set the defaultUploadLocationSource to null, which will break the field.

Schematic will look for a volume with ID 3, but there isn't one, there's only a volumefolder with ID 3. Now there isn't any issue as long as the volumefolders and volumes have exactly the same ID's, but as soon as you create more folders or the ID's don't match anymore (for example volumefolder 'home' has ID 2, but volume 'home' has ID 3) this will mess up the assets/database.

This will happen often, especially with bigger websites, so this is a major bug I guess.

Possible fix

We noticed that the ElementIndexMapper converts IDs to handles and back, probably for readability in the schema.yml file?

The Assets converter (Converters/Fields/Assets.php) goes from ID to handle on export and from handle to ID on import, problem is though: only volumes have handles and volumefolders don't.

So... going from handle to ID and viceversa isn't possible here? It looks at the volume handle and tries to store the volume ID instead of the volumefolder ID. Once we change the Assets converter to go from ID to ID, ~it's fixed~ (see comment below).


Now, we aren't sure if this is the best way of fixing this. It's probably better that you guys look at this, because you know schematic inside out ;)

Thanks!

thijskaspers commented 6 years ago

An update: seems that from ID to ID isn't the fix.. it still uses the volume ID, instead of the volumefolder ID, resulting in the same errors:

No mapping found for source folder:9
No mapping found for source folder:6

And the defaultUploadLocationSource of the field will be set to null which will break the field or set it to the first volume it can find

thijskaspers commented 6 years ago

Think I found the fix: It also needs this in Behaviors\SourcesBehavior.php

case 'folder':
    $service = Craft::$app->assets;
    $method = 'getFolderBy';
    break;

And from ID to ID on both export and import in Converters\Fields\Assets.php

bvangennep commented 6 years ago

Thanks for looking into this. Ideally we don't want any id's in the schema because this can differ between environments.

grumpyoldman-io commented 6 years ago

This bug also pops up when using a remote folder (AWS S3).

RobinDevelopersNL commented 5 years ago

Is there any ETA on this?

We have the same situation where our volumeID doesn't correspond with the ID of the folder. Craft has created a temporary source which breaks the ID coupling. Our volume has ID 5, folder has ID 10. We get No mapping found for source, in both export and import.

bvangennep commented 5 years ago

working on it. https://github.com/nerds-and-company/schematic/pull/160

RobinDevelopersNL commented 5 years ago

Nice, thank you!

thijskaspers commented 5 years ago

Thanks @bvangennep!