Open
Conversation
added 2 commits
September 23, 2015 15:55
Reason is issue b13#62 - DAM may have multiple records for the same file. Since we correlate DAM and FAL by file path, we need to associate at least one DAM record to each migrated FAL record, thus the old way of using one field sys_file._migrateddamuid was incomplete as it only allowed associating at most one DAM record to each FAL record. Note that we may have different metadata for the same file as a result. Before these changes, only the metadata of the last file having been migrated was used. Now, all metadata will be processed which may lead to errors. This commit is only intended to record the change in columns and does not handle that issue yet.
If multiple metadata for same files is encountered, we now print a warning prompting the user to check migrated metadata of all listed files (message includes file path and UIDs of sys_file and tx_dam records). This is for issue b13#62 again but still doesn't attempt to solve the conflict, we leave it up to the user to check for any possible errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sys_file._migrateddamuidis being replaced bytx_dam._migratedfaluidto prevent loosing references when we got multiple DAM records for the same file. See issue #62 for a detailed explanation of the problem.DAM may have multiple records for the same file. Since we correlate DAM and FAL by file path, we need to associate at least one DAM record to each migrated FAL record, thus the old way of using one field
sys_file._migrateddamuidwas incomplete as it only allowed associating at most one DAM record to each FAL record.If multiple metadata for same files is encountered, we now print a warning prompting the user to check migrated metadata of all listed files (message includes file path and UIDs of
sys_fileandtx_damrecords).
After these changes, all metadata records will be processed without any specific order. This should not cause any worse effects than before (previously, metadata migration ran only once per file but already used a more or less random DAM record as source).
Important
This changes an essential part of dam_falmigration. Migration for a 400 page website using tt_news and DAM categories ran successfully for us. Nevertheless, if possible, please perform a code review of these changes before accepting the pull request to check for any mistakes we did not see. Please pay special attention to the migration code for
tx_damdownloadlist_records(DamMigrationCommandController.phplines 376-378) as it could not be tested on our website as it does not use that extension.