diff --git a/docs/en/09_migrating/01_linkable-migration.md b/docs/en/09_migrating/01_linkable-migration.md index 2bb4a308..fa309f4b 100644 --- a/docs/en/09_migrating/01_linkable-migration.md +++ b/docs/en/09_migrating/01_linkable-migration.md @@ -1,151 +1,364 @@ --- -title: Migrating from Shae Dawson's Linkable module +title: Migrating from sheadawson/silverstripe-linkable module summary: A guide for migrating from sheadawson/silverstripe-linkable to silverstripe/linkfield --- -# Migrating from Shae Dawson's Linkable module +# Migrating from sheadawson/silverstripe-linkable module -The [`sheadawson/silverstripe-linkable` module](https://github.com/sheadawson/silverstripe-linkable) was a much loved, and much used module. It is, unfortunately, no longer maintained. We have provided some steps and tasks that we hope can be used to migrate your project from Linkable to LinkField. +> [!NOTE] +> If your site is running Silverstripe CMS 4.x, upgrade to CMS 5 first. +> You will most likely need to use a fork of `sheadawson/silverstripe-linkable` that is compatible with Silverstripe CMS 5 as part of this upgrade. +> Once you have finished upgrading to CMS 5, return to this guide and continue the linkfield upgrade. -> [!WARNING] -> This guide and the associated migration task assume all of the data for your links are in the base table for `Sheadawson\Linkable\Models\Link` or in automatically generated tables (e.g. join tables for `many_many` relations). -> If you have subclassed `Sheadawson\Linkable\Models\Link`, there may be additional steps you need to take to migrate the data for your subclass. +The [`sheadawson/silverstripe-linkable` module](https://github.com/sheadawson/silverstripe-linkable) was a much loved, and much used module. It is, unfortunately, no longer maintained. We have provided some steps and tasks that we hope can be used to migrate your project from Linkable to LinkField. -## Preamble +There are a few major changes between `sheadawson/silverstripe-linkable` and `silverstripe/linkfield`: -This migration process covers shifting data from the `Linkable` tables to the appropriate `LinkField` tables. This does not cover usages of `EmbeddedObject`. +1. Link types are defined via subclasses in `silverstripe/linkfield` as opposed to configuration within a single model. +1. `silverstripe/linkfield` doesn't support `many_many` relations - these will be migrated to `has_many` relations instead. +1. Many fields and relations have different names. +1. The default title for a link isn't stored in the database - if the `LinkText` field is left blank, nothing gets stored in the database for that field. + - This means any links migrated which had the default title set will be migrated with that title as explicit `LinkText`, which will not update automatically when you change the link URL. + - If you want the `LinkText` for those links to update automatically, you will need to either [customise the migration](#customising-the-migration) or manually unset the `LinkText` for those links afterward. -**Versioned:** If you have `Versioned` `Linkable`, then the expectation is that you will also `Version` `LinkField`. If you have not `Versioned` `Linkable`, then the expectation is that you will **not** `Version` `LinkField`. +The following additional items were identified as feature gaps, which may require some additional work to implement if you require them: -**No support for internal links with query params (GET params):** Please be aware that Linkfield does not support internal links with query params (`?`) out of the box, and therefor the migration task will **remove** any query params that are present in the Linkable's `Anchor` field. +- Adding custom class to link. The `setCSSClass()` method does not exist in Linkfield. But you can still can add an Extension to the Link class or develop your own custom link class and implement the logic of this method. +- Customizing link templates. You can still call the `renderWith()` method and pass the name of your custom template, but `LinkField` no longer supports `templates` configuration. +- Limit allowed Link types. The `LinkField` module does not support the `allowed_types` configuration. Now, in order to set a limited list of link types available to the user, you should use the `Link::setAllowedTypes()` method. +- Custom query params. This functionality is not supported. You can no longer set the `data-extra-query` attribute to a link. But you can still add an extension to the link and template that will allow you to implement this logic. +- `EmbeddedObject` field is not supported and you can no longer use the `EmbeddedObject` class as part of a LinkField. +- If you have subclassed `Sheadawson\Linkable\Models\Link`, there may be additional steps you need to take to migrate the data for your subclass. -## Install Silvesrtripe Linkfield +> [!WARNING] +> This guide and the associated migration task assume all of the data for your links are in the base table for `Sheadawson\Linkable\Models\Link` or in automatically generated tables (e.g. join tables for `many_many` relations). -Install the Silverstripe Linkfield module: +## Setup -```bash -composer require silverstripe/linkfield 4 -``` +> [!TIP] +> We strongly recommend taking a backup of your database before doing anything else. +> This will ensure you have a known state to revert to in case anything goes wrong. -Optionally, you can also remove the Linkable module (though, you might find it useful to keep around as a reference while you are upgrading your code). +### Update your dependencies -Do this step at whatever point makes sense to you. +Remove the Linkable module and add `silverstripe/linkfield`: ```bash composer remove sheadawson/silverstripe-linkable +composer require silverstripe/linkfield:^4 ``` -## Replace app usages +### Configure the migration task + +> [!NOTE] +> Be sure to check how the old module classes are referenced in config `yml` files (eg: `app/_config`). Update appropriately. + +1. Enable the task: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + is_enabled: true + ``` + +1. Declare any columns that you added to the linkable link model which need to be migrated to the new base link table, for example if you added a custom sort column for your `has_many` relations: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + base_link_columns: + MySortColumn: 'Sort' + ``` + +1. Declare any `has_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + has_many_links_data: + # The class where the has_many relation is declared + App\Model\MyClass: + # The key is the name of the has_many relation + # The value is the name of the old has_one relation on the Linkable link model + LinkListOne: 'MyOwner' + ``` + +1. Declare any `many_many` relations that need to be migrated: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + many_many_links_data: + # The class where the many_many relation is declared + App\Model\MyClass: + # If it's a normal many_many relation with no extra fields, + # you can simply set the value to null and let the migration task figure it out + LinkListExample: null + # If the many_many is a many_many through, or had a $many_many_extraFields configuration defined, + # you will need to provide additional information + LinkListTwo: + # The table is required for many_many through + table: 'Page_ManyManyLinks' + # Extra fields is for $many_many_extraFields, or for any $db fields on a + # many_many through join model + extraFields: + MySort: 'Sort' + # For many_many through relations, you must add the names of the has_one relations + # from the DataObject which was used as the join model + through: + from: 'FromHasOneName', + to: 'ToHasOneName', + ``` + +1. Declare any classes that may have `has_one` relations to `Link`, but which do not *own* the link. Classes declared here will include any subclasses. + For example if a custom link has a `has_many` relation to some class which does not own the link, declare that class here so it is not incorrectly identified as the owner of the link: + + ```yml + SilverStripe\LinkField\Tasks\LinkableMigrationTask: + # ... + classes_that_are_not_link_owners: + - App\Model\SomeClass + ``` + +### Update your codebase + +You should review how you are using the original `Link` model and `LinkField`, but if you don't have any customisations, then replacing the old with the new might be quite simple. + +1. If you added any database columns to the `Link` class for sorting `has_many` relations, or any `has_one` relations for storing them, remove the extension or yaml configuration for that now. + + ```diff + - Sheadawson\Linkable\Models\Link: + - db: + - MySortColumn: Int + - has_one: + - MyOwner: App\Model\MyClass + - belongs_many_many: + - BelongsRecord : App\Model\MyClass.LinkListTwo + ``` + +1. Update use statements and relations for the classes which own links. + - Any `many_many` relations should be swapped out for `has_many` relations, and all `has_many` relations should point to the `Owner` relation on the link class via dot notation. + - If the models that have `has_one` or `has_many` relations to link don't already use the `$owns` configuration for those relations, add that now. You may also want to set `$cascade_deletes` and `$cascade_duplicates` configuration. See [basic usage](../01_basic_usage.md) for more details.ed. + + ```diff + namespace App\Model; + + - use Sheadawson\Linkable\Models\Link; + - use Sheadawson\Linkable\Forms\LinkField; + + use SilverStripe\LinkField\Models\Link; + + use SilverStripe\LinkField\Form\LinkField; + + use SilverStripe\LinkField\Form\MultiLinkField; + use SilverStripe\ORM\DataObject; + + class MyClass extends DataObject + { + private static array $has_one = [ + 'HasOneLink' => Link::class, + ]; + + private static array $has_many = [ + - 'LinkListOne' => Link::class . '.MyOwner', + + 'LinkListOne' => Link::class . '.Owner', + + 'LinkListTwo' => Link::class . '.Owner', + ]; + + + private static array $owns = [ + + 'HasOneLink', + + 'LinkListOne', + + 'LinkListTwo', + + ]; + + + - private static array $many_many = [ + - 'LinkListTwo' => Link::class, + - ]; + - + - private static array $many_many_extraFields = [ + - 'LinkListTwo' => [ + - 'MySort' => 'Int', + - ] + - ]; + } + ``` + +1. If you had `many_many` through relation, delete the `DataObject` class which was used as the join table. +1. Update the usage of link field (and `GridField` if you were using that to manage `has_many` or `many_many` relations). + - Note that the linkable module's `LinkField` required you to specify the related field with `ID` appended (e.g. `HasOneLinkID`), whereas the new `LinkField` requires you to specify the field without `ID` appended (e.g. `HasOneLink`). + + ```diff + public function getCMSFields() + { + $fields = parent::getCMSFields(); + + $fields->removeByName(['HasOneLinkID', 'LinkListOne', 'LinkListTwo']); + $fields->addFieldsToTab( + 'Root.Main', + [ + - LinkField::create('HasOneLinkID', 'Has one link') + + LinkField::create('HasOneLink', 'Has one link'), + - GridField::create( + - 'LinkListTwo', + - 'Link List Two', + - $this->LinkListTwo(), + - GridFieldConfig_RelationEditor::create() + - ->removeComponentsByType(GridFieldAddExistingAutocompleter::class) + - ), + + MultiLinkField::create('LinkListOne', 'List list one'), + + MultiLinkField::create('LinkListTwo', 'List list two'), + ] + ); + return $fields; + } + + ``` + +1. If you applied [linkfield configuration](https://github.com/elliot-sawyer/silverstripe-linkfield?tab=readme-ov-file#configuration), update that now. + - See [configuring links and link fields](../02_configuration.md) for more information. + + ```diff + + use SilverStripe\LinkField\Models\ExternalLink; + + use SilverStripe\LinkField\Models\SiteTreeLink; + + - $linkConfig = [ + - 'types' => [ + - 'SiteTree', + - 'URL', + - ], + - 'title_display' => false, + - ]; + - $linkField->setLinkConfig($linkConfig); + + $allowedTypes = [ + + SiteTreeLink::class, + + ExternalLink::class, + + ]; + + $linkField->setAllowedTypes($allowedTypes); + + $linkField->setExcludeLinkTextField(true); + ``` -You should review how you are using the original `Link` model and `LinkField`, but if you don't have any customisations, then replacing the old with the new **might** be quite simple. +### Populate module -If you have used imports (`use` statements), then your first step might just be to search for `use [old];` and replace -with `use [new];` (since the class name references have not changed at all). -```diff -- Sheadawson\Linkable\Models\Link -+ SilverStripe\LinkField\Models\Link +If you use the `dnadesign/silverstripe-populate` module, you will not be able to simply "replace" the namespace. Fixture definitions for the new Linkfield module are quite different. There are entirely different models for different link types, whereas before it was just a DB field to specify the type. -- Sheadawson\Linkable\Forms\LinkField -+ SilverStripe\LinkField\Form\LinkField +See below for example: + +```diff +- Sheadawson\Linkable\Models\Link: +- internal: +- Title: Internal link +- Type: SiteTree +- SiteTreeID: 1 +- OpenInNewWindow: true ++ SilverStripe\LinkField\Models\SiteTreeLink: ++ internal: ++ LinkText: Internal link ++ Page: =>Page.home ++ OpenInNew: true +- external: +- Title: External link +- Type: URL +- URL: https://example.org +- OpenInNewWindow: true ++ SilverStripe\LinkField\Models\ExternalLink: ++ external: ++ LinkText: External link ++ ExternalUrl: https://example.org ++ OpenInNew: true +- file: +- Title: File link +- Type: File +- File: =>SilverStripe\Assets\File.example ++ SilverStripe\LinkField\Models\FileLink: ++ file: ++ LinkText: File link ++ File: =>SilverStripe\Assets\File.example +- phone: +- Title: Phone link +- Type: Phone +- Phone: +64 1 234 567 ++ SilverStripe\LinkField\Models\PhoneLink: ++ phone: ++ LinkText: Phone link ++ Phone: +64 1 234 567 +- email: +- Title: Email link +- Type: Email +- Email: foo@example.org ++ SilverStripe\LinkField\Models\EmailLink: ++ email: ++ LinkText: Email link ++ Email: foo@example.org ``` -If you have extensions, new fields, etc, then your replacements might need to be a bit more considered. +## Replace template usages -The other key (less easy to automate) thing that you'll need to update is that the old `LinkField` required you to specify the related field with `ID` appended, whereas the new `LinkField` requires you to specify the field without `ID` appended. EG. -```diff -- LinkField::create('MyLinkID') -+ LinkField::create('MyLink') -``` -Search for instances of `LinkField::create` and `new LinkField`, and hopefully that should give you all of the places where you need to update field name references. +The link element structure is rendered using the `Link.ss` template. You can override this template by copying it to the theme or project folder and making the necessary changes. You still can also specify a custom template to display the link by using the `renderWith()` method and passing the name of your custom template. -### Configuration +Also, when working on your own template, you should consider the following differences in variable names. -Be sure to check how the old module classes are referenced in config `yml` files (eg: `app/_config`). Update appropriately. +**Before:** You might have had references to `$LinkURL` or `$Link.LinkURL`.\ +**After:** These would need to be updated to `$URL` or `$Link.URL` respectively. -### Populate module +**Before:** `$OpenInNewWindow` or `$Link.OpenInNewWindow`.\ +**After:** `$OpenInNew` or `$Link.OpenInNew` respectively. -If you use the populate module, you will not be able to simply "replace" the namespace. Fixture definitions for the new Linkfield module are quite different. There are entirely different models for different link types, whereas before it was just a DB field to specify the type. +**Before:** `$Link.TargetAttr` or `$TargetAttr` would output the appropriate `target="xx"`.\ +**After:** There is no direct replacement. -See below for example before/after usage: +## Customising the migration -#### Before +### Custom links -```yml -Sheadawson\Linkable\Models\Link: - internal: - Title: Internal link - Type: SiteTree - SiteTreeID: 1 - OpenInNewWindow: true - external: - Title: External link - Type: URL - URL: https://example.org - OpenInNewWindow: true - file: - Title: File link - Type: File - File: =>SilverStripe\Assets\File.example - phone: - Title: Phone link - Type: Phone - Phone: +64 1 234 567 - email: - Title: Email link - Type: Email - Email: foo@example.org -``` +If you have custom link implementations, you will need to implement an appropriate subclass of [`Link`](api:SilverStripe\LinkField\Models\Link) (or apply an extension to an existing one) with appropriate database columns and relations. -#### After +You need to update configuration `LinkableMigrationTask` so it knows how to handle the migration from the old link to the new one: ```yml -SilverStripe\LinkField\Models\SiteTreeLink: - internal: - LinkText: Internal link - Page: =>Page.home - OpenInNew: true -SilverStripe\LinkField\Models\ExternalLink: - external: - LinkText: External link - ExternalUrl: https://example.org - OpenInNew: true -SilverStripe\LinkField\Models\FileLink: - file: - LinkText: File link - File: =>SilverStripe\Assets\File.example -SilverStripe\LinkField\Models\PhoneLink: - phone: - LinkText: Phone link - Phone: +64 1 234 567 -SilverStripe\LinkField\Models\EmailLink: - email: - LinkText: Email link - Email: foo@example.org +SilverStripe\LinkField\Tasks\GorriecoeMigrationTask: + # ... + link_type_columns: + # The name of the Type for your custom type as defined in ===== + MyCustomType: + # The FQCN for your new custom link subclass + class: 'App\Model\Link\MyCustomLink' + # An mapping of column names from the old link to your new link subclass + # Only include columns that are defined in the $db configuration for your subclass + fields: + MyOldField: 'MyNewField' ``` -## Replace template usages - -**Before:** You might have had references to `$LinkURL` or `$Link.LinkURL`. -**After:** These would need to be updated to `$URL` or `$Link.URL` respectively. - -**Before:** `$OpenInNewWindow` or `$Link.OpenInNewWindow`. -**After:** `$OpenInNew` or `$Link.OpenInNew` respectively. - -**Before:** `$Link.TargetAttr` or `$TargetAttr` would output the appropriate `target="xx"`. -**After:** There is no direct replacement. +## Migrating -This is an area where you should spend some decent effort to make sure each implementation is outputting as you expect it to. There may be more "handy" methods that Linkable provided that no longer exist (that we haven't covered above). +> [!NOTE] +> This migration process covers shifting data from the `LinkableLink` tables to the appropriate `LinkField` tables. -## Table structures +For databases that support transactions, the full data migration is performed within a single transaction, and any errors in the migration will result in rolling back all changes. This means you can address whatever caused the error and then run the task again. -It's important to understand that we are going from a single table in Linkable to multiple tables in LinkField. +> [!NOTE] +> We strongly recommend running this task in a local development environment before trying it in production. +> There may be edge cases that the migration task doesn't account for which need to be resolved. -**Before:** We had 1 table with all data, and one of the field in there specified the type of the Link. -**After:** We have 1 table for each type of Link, with a base `Link` table for all record. +1. Run dev/build and flush your cache (use the method you will be using the for next step - i.e. if you're running the task via the terminal, make sure to flush via the terminal) + - via the browser: `https://www.example.com/dev/build?flush=1` + - via a terminal: `sake dev/build flush=1` +1. Run the task + - via the browser: `https://www.example.com/dev/tasks/linkable-to-linkfield-migration-task` + - via a terminal: `sake dev/tasks/linkable-to-linkfield-migration-task` -## Migrating +The task performs the following steps: -The migration process completely repeats the process of updating the old version of Linkfield to the new one, so please follow the instructions provided in the [Migrating section](./00_upgrading.md#migrating) making the following small changes to the configuration files. +1. Inserts new rows into the base link table, taking values from the old link table. +1. Inserts new rows into tables for link subclasses, taking values from the old link table. +1. Updates `SiteTreeLink` records, splitting out the old `Anchor` column into the separate `Anchor` and `QueryString` columns. +1. Migrates any `has_many` relations which were declared in [`LinkableMigrationTask.has_many_links_data`](api:SilverStripe\LinkField\Tasks\LinkableMigrationTask->has_many_links_data). +1. Migrates any `many_many` relations which were declared in in [`LinkableMigrationTask.many_many_links_data`](api:SilverStripe\LinkField\Tasks\LinkableMigrationTask->many_many_links_data) and drops the old join tables. +1. Set the `Owner` relation for `has_one` relations to links. +1. Drops the old link table. +1. Publishes all links, unless you have removed the `Versioned` extension. +1. Output a table with any links which are lacking the data required to generate a URL. + - You can skip this step by adding `?skipBrokenLinks=1` to the end of the URL: `https://www.example.com/dev/tasks/linkable-to-linkfield-migration-task?skipBrokenLinks=1`. + - If you're running the task in a terminal, you can add `skipBrokenLinks=1` as an argument: `sake dev/tasks/linkable-to-linkfield-migration-task skipBrokenLinks=1`. -- Change Task class name from `SilverStripe\LinkField\Tasks\LinkFieldMigrationTask` to `SilverStripe\LinkField\Tasks\LinkableMigrationTask` in each yml configuration file. +> [!WARNING] +> If the same link appears in multiple `many_many` relation lists within the same relation (with different owner records), the link will be duplicated so that a single link exists for each `has_many` relation list. +> +> Unless you were doing something custom to manage links it's unlikely this will affect you - but if it does, just be aware of this and prepare your content authors for this change in their authoring workflow. +> +> If the same link appears in multiple `many_many` relation lists across *different* relations, you will need to handle the migration of this scenario yourself. The migration task will not duplicate these links. The link's owner will be whichever record is first identified, and any further owner records will simply not have that link in their `has_many` relation list. diff --git a/docs/en/09_migrating/02_gorriecoe-migration.md b/docs/en/09_migrating/02_gorriecoe-migration.md index 829dc554..0277ae8f 100644 --- a/docs/en/09_migrating/02_gorriecoe-migration.md +++ b/docs/en/09_migrating/02_gorriecoe-migration.md @@ -54,6 +54,9 @@ composer require silverstripe/linkfield:^4 ### Configure the migration task +> [!NOTE] +> Be sure to check how the old module classes are referenced in config `yml` files (eg: `app/_config`). Update appropriately. + 1. Enable the task: ```yml @@ -122,6 +125,8 @@ composer require silverstripe/linkfield:^4 ### Update your codebase +You should review how you are using the original `Link` model and `LinkField`, but if you don't have any customisations, then replacing the old with the new might be quite simple. + 1. If you added any database columns to the `Link` class for sorting `has_many` relations, or any `has_one` relations for storing them, remove the extension or yaml configuration for that now. ```diff @@ -277,8 +282,8 @@ For databases that support transactions, the full data migration is performed wi - via the browser: `https://www.example.com/dev/build?flush=1` - via a terminal: `sake dev/build flush=1` 1. Run the task - - via the browser: `https://www.example.com/dev/tasks/linkfield-tov4-migration-task` - - via a terminal: `sake dev/tasks/linkfield-tov4-migration-task` + - via the browser: `https://www.example.com/dev/tasks/gorriecoe-to-linkfield-migration-task` + - via a terminal: `sake dev/tasks/gorriecoe-to-linkfield-migration-task` The task performs the following steps: diff --git a/src/Tasks/GorriecoeMigrationTask.php b/src/Tasks/GorriecoeMigrationTask.php index 4519fec7..40e967e9 100644 --- a/src/Tasks/GorriecoeMigrationTask.php +++ b/src/Tasks/GorriecoeMigrationTask.php @@ -3,20 +3,14 @@ namespace SilverStripe\LinkField\Tasks; use RuntimeException; -use SilverStripe\Core\ClassInfo; use SilverStripe\Dev\BuildTask; +use SilverStripe\LinkField\Models\Link; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; use SilverStripe\LinkField\Models\FileLink; -use SilverStripe\LinkField\Models\Link; use SilverStripe\LinkField\Models\PhoneLink; use SilverStripe\LinkField\Models\SiteTreeLink; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; -use SilverStripe\ORM\Queries\SQLDelete; -use SilverStripe\ORM\Queries\SQLSelect; -use SilverStripe\ORM\Queries\SQLUpdate; -use SilverStripe\Versioned\Versioned; /** * @deprecated 4.0.0 Will be removed without equivalent functionality. @@ -99,66 +93,37 @@ class GorriecoeMigrationTask extends BuildTask ]; /** - * List any has_many relations that should be migrated. - * - * Keys are the FQCN for the class where the has_many is declared. - * Values are the name of the old has_one. - * - * Example: - * - * // SiteConfig had two has_many relationships, - * // one to Link.MyHasOne and another to Link.DifferentHasOne. - * SiteConfig::class => [ - * 'LinksListOne' => 'MyHasOne', - * 'LinksListTwo' => 'DifferentHasOne', - * ] - * + * Perform the actual data migration and publish links as appropriate */ - private static array $has_many_links_data = []; + public function performMigration(): void + { + $this->extend('beforePerformMigration'); + // Because we're using SQL INSERT with specific ID values, + // we can't perform the migration if there are existing links because there + // may be ID conflicts. + if (Link::get()->exists()) { + throw new RuntimeException('Cannot perform migration with existing silverstripe/linkfield link records.'); + } - /** - * List any many_many relations that should be migrated. - * - * Keys are the FQCN for the class where the many_many is declared. See example below for values. - * - * Example: - * - * // SiteConfig had three many_many relationships: - * // The table name for "LinksListOne" will be guessed. It wasn't a many_many through and had no extra fields - * // The table name for "LinksListTwo" will be guessed. It wasn't a many_many through but did have some extra fields - * // The table name for "LinksListThree" is explicitly provided. It was a many_many through with some extra fields - * SiteConfig::class => [ - * 'LinksListOne' => null, - * 'LinksListTwo' => [ - * 'extraFields' => [ - * 'MySort' => 'Sort', - * ], - * ], - * 'LinksListThree' => [ - * 'table' => 'App_MyThroughClassTable', - * 'extraFields' => [ - * 'MySort' => 'Sort', - * ], - * 'through' => [ - * 'from' => 'FromHasOneName', - * 'to' => 'ToHasOneName', - * ], - * ], - * ] - * - */ - private static array $many_many_links_data = []; + $this->insertBaseRows(); + $this->insertTypeSpecificRows(); + $this->updateSiteTreeRows(); + $this->migrateHasManyRelations(); + $this->migrateManyManyRelations(); + $this->setOwnerForHasOneLinks(); - /** - * The table name for the base gorriecoe link model. - */ - private string $oldTableName; + $this->print("Dropping old link table '{$this->oldTableName}'"); + DB::get_conn()->query("DROP TABLE \"{$this->oldTableName}\""); - /** - * The old link model class name - */ - private function classIsOldLink(string $class): bool - { - return $class === 'gorriecoe\Link\Models\Link'; + $this->print('-----------------'); + $this->print('Bulk data migration complete. All links should be correct (but unpublished) at this stage.'); + $this->print('-----------------'); + + $this->publishLinks(); + + $this->print('-----------------'); + $this->print('Migration completed successfully.'); + $this->print('-----------------'); + $this->extend('afterPerformMigration'); } } diff --git a/src/Tasks/LinkFieldMigrationTask.php b/src/Tasks/LinkFieldMigrationTask.php index 548b27da..01f0ac12 100644 --- a/src/Tasks/LinkFieldMigrationTask.php +++ b/src/Tasks/LinkFieldMigrationTask.php @@ -278,9 +278,4 @@ private function migrateHasManyRelations(): void } $this->extend('afterMigrateHasManyRelations'); } - - private function classIsOldLink(string $class): bool - { - return is_a($class, Link::class, true); - } } diff --git a/src/Tasks/LinkableMigrationTask.php b/src/Tasks/LinkableMigrationTask.php index 44d03210..242f83bd 100644 --- a/src/Tasks/LinkableMigrationTask.php +++ b/src/Tasks/LinkableMigrationTask.php @@ -2,12 +2,15 @@ namespace SilverStripe\LinkField\Tasks; +use RuntimeException; use SilverStripe\Dev\BuildTask; +use SilverStripe\LinkField\Models\Link; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; use SilverStripe\LinkField\Models\FileLink; use SilverStripe\LinkField\Models\PhoneLink; use SilverStripe\LinkField\Models\SiteTreeLink; +use SilverStripe\ORM\DB; /** * @deprecated 4.0.0 Will be removed without equivalent functionality. @@ -17,7 +20,7 @@ class LinkableMigrationTask extends BuildTask use MigrationTaskTrait; use ModuleMigrationTaskTrait; - private static $segment = 'linkable-migration-task'; + private static $segment = 'linkable-to-linkfield-migration-task'; protected $title = 'Linkable to Linkfield Migration Task'; @@ -26,7 +29,7 @@ class LinkableMigrationTask extends BuildTask /** * Enable via YAML configuration if you need to run this task */ - private static ?bool $is_enabled = true; + private static ?bool $is_enabled = false; /** * The number of links to process at once, for operations that operate on each link individually. @@ -45,7 +48,7 @@ class LinkableMigrationTask extends BuildTask /** * Mapping for columns in the base link table. - * Doesn't include + * Doesn't include subclass columns - see link_type_columns */ private static array $base_link_columns = [ 'OpenInNewWindow' => 'OpenInNew', @@ -90,26 +93,37 @@ class LinkableMigrationTask extends BuildTask ]; /** - * List any has_many relations that should be migrated. + * Perform the actual data migration and publish links as appropriate */ - private static array $has_many_links_data = []; - - - /** - * List any many_many relations that should be migrated. - */ - private static array $many_many_links_data = []; - - /** - * The table name for the base link model. - */ - private string $oldTableName; - - /** - * The old link model class name - */ - private function classIsOldLink(string $class): bool + public function performMigration(): void { - return $class === 'Sheadawson\Linkable\Models\Link'; + $this->extend('beforePerformMigration'); + // Because we're using SQL INSERT with specific ID values, + // we can't perform the migration if there are existing links because there + // may be ID conflicts. + if (Link::get()->exists()) { + throw new RuntimeException('Cannot perform migration with existing silverstripe/linkfield link records.'); + } + + $this->insertBaseRows(); + $this->insertTypeSpecificRows(); + $this->updateSiteTreeRows(); + $this->migrateHasManyRelations(); + $this->migrateManyManyRelations(); + $this->setOwnerForHasOneLinks(); + + $this->print("Dropping old link table '{$this->oldTableName}'"); + DB::get_conn()->query("DROP TABLE \"{$this->oldTableName}\""); + + $this->print('-----------------'); + $this->print('Bulk data migration complete. All links should be correct (but unpublished) at this stage.'); + $this->print('-----------------'); + + $this->publishLinks(); + + $this->print('-----------------'); + $this->print('Migration completed successfully.'); + $this->print('-----------------'); + $this->extend('afterPerformMigration'); } } diff --git a/src/Tasks/MigrationTaskTrait.php b/src/Tasks/MigrationTaskTrait.php index 3ce768cd..c9c66d69 100644 --- a/src/Tasks/MigrationTaskTrait.php +++ b/src/Tasks/MigrationTaskTrait.php @@ -126,7 +126,7 @@ private function setOwnerForHasOneLinks(): void } // Skip if the has_one isn't for Link, or points at a belongs_to or has_many on Link - if (!$this->classIsOldLink($hasOneClass)) { + if (!is_a($hasOneClass, Link::class, true)) { continue; } if ($this->hasReciprocalRelation([$hasOneClass], $hasOneName, $modelClass)) { @@ -435,9 +435,4 @@ abstract public function performMigration(): void; * Check if we actually need to migrate anything, and if not give clear output as to why not. */ abstract private function getNeedsMigration(): bool; - - /** - * Returns true if the class represents an old link to be migrated - */ - abstract private function classIsOldLink(string $class): bool; } diff --git a/src/Tasks/ModuleMigrationTaskTrait.php b/src/Tasks/ModuleMigrationTaskTrait.php index 2f8a07c6..438e65f0 100644 --- a/src/Tasks/ModuleMigrationTaskTrait.php +++ b/src/Tasks/ModuleMigrationTaskTrait.php @@ -19,39 +19,60 @@ trait ModuleMigrationTaskTrait { /** - * Perform the actual data migration and publish links as appropriate + * List any has_many relations that should be migrated. + * + * Keys are the FQCN for the class where the has_many is declared. + * Values are the name of the old has_one. + * + * Example: + * + * // SiteConfig had two has_many relationships, + * // one to Link.MyHasOne and another to Link.DifferentHasOne. + * SiteConfig::class => [ + * 'LinksListOne' => 'MyHasOne', + * 'LinksListTwo' => 'DifferentHasOne', + * ] + * */ - public function performMigration(): void - { - $this->extend('beforePerformMigration'); - // Because we're using SQL INSERT with specific ID values, - // we can't perform the migration if there are existing links because there - // may be ID conflicts. - if (Link::get()->exists()) { - throw new RuntimeException('Cannot perform migration with existing silverstripe/linkfield link records.'); - } - - $this->insertBaseRows(); - $this->insertTypeSpecificRows(); - $this->updateSiteTreeRows(); - $this->migrateHasManyRelations(); - $this->migrateManyManyRelations(); - $this->setOwnerForHasOneLinks(); - - $this->print("Dropping old link table '{$this->oldTableName}'"); - DB::get_conn()->query("DROP TABLE \"{$this->oldTableName}\""); - - $this->print('-----------------'); - $this->print('Bulk data migration complete. All links should be correct (but unpublished) at this stage.'); - $this->print('-----------------'); + private static array $has_many_links_data = []; - $this->publishLinks(); + /** + * List any many_many relations that should be migrated. + * + * Keys are the FQCN for the class where the many_many is declared. See example below for values. + * + * Example: + * + * // SiteConfig had three many_many relationships: + * // The table name for "LinksListOne" will be guessed. It wasn't a many_many through and had no extra fields + * // The table name for "LinksListTwo" will be guessed. It wasn't a many_many through but did have some extra fields + * // The table name for "LinksListThree" is explicitly provided. It was a many_many through with some extra fields + * SiteConfig::class => [ + * 'LinksListOne' => null, + * 'LinksListTwo' => [ + * 'extraFields' => [ + * 'MySort' => 'Sort', + * ], + * ], + * 'LinksListThree' => [ + * 'table' => 'App_MyThroughClassTable', + * 'extraFields' => [ + * 'MySort' => 'Sort', + * ], + * 'through' => [ + * 'from' => 'FromHasOneName', + * 'to' => 'ToHasOneName', + * ], + * ], + * ] + * + */ + private static array $many_many_links_data = []; - $this->print('-----------------'); - $this->print('Migration completed successfully.'); - $this->print('-----------------'); - $this->extend('afterPerformMigration'); - } + /** + * The table name for the base gorriecoe link model. + */ + private string $oldTableName; /** * Check if we actually need to migrate anything, and if not give clear output as to why not. diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.php b/tests/php/Tasks/GorriecoeMigrationTaskTest.php index 72bbfb97..698fcda3 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.php +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.php @@ -6,7 +6,6 @@ use ReflectionMethod; use ReflectionProperty; use RuntimeException; -use SilverStripe\Dev\Debug; use SilverStripe\Dev\SapphireTest; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; @@ -20,6 +19,7 @@ use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner; use SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\CustomLink; use SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\HasManyLinkOwner; +use SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasHasOneLinkableLinkOwner; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBField; @@ -49,6 +49,7 @@ class GorriecoeMigrationTaskTest extends SapphireTest LinkOwner::class, WasManyManyJoinModel::class, WasManyManyOwner::class, + WasHasOneLinkableLinkOwner::class, ]; /** @@ -576,6 +577,82 @@ public function testMigrateManyManyRelationsExceptions(array $config, string $ex } } + public function testSetOwnerForHasOneLinks(): void + { + $this->startCapturingOutput(); + $this->callPrivateMethod('setOwnerForHasOneLinks'); + $output = $this->stopCapturingOutput(); + + $ownerClass = WasHasOneLinkableLinkOwner::class; + $fixtureRelationsHaveLink = [ + 'owner-1' => ['Link' => true], + 'owner-2' => ['Link' => true], + 'owner-3' => ['Link' => true], + ]; + + $relationsByID = []; + foreach ($fixtureRelationsHaveLink as $fixture => $data) { + $data['fixture'] = $fixture; + $relationsByID[$this->idFromFixture($ownerClass, $fixture)] = $data; + } + + foreach (DataObject::get($ownerClass) as $owner) { + if (array_key_exists($owner->ID, $relationsByID)) { + $data = $relationsByID[$owner->ID]; + $ownerFixture = $data['fixture']; + $record = $this->objFromFixture($ownerClass, $ownerFixture); + foreach ($data as $relation => $hasLink) { + if ($relation === 'fixture') { + continue; + } + /** @var Link $link */ + $link = $record->$relation(); + if ($hasLink === null) { + // Relation should either not be for link, or should not be in the DB. + if (is_a($link->ClassName, Link::class, true)) { + $this->assertFalse($link->isInDB(), "Relation {$relation} should not have a link saved in it"); + } + continue; + } elseif ($hasLink === false) { + // The special case is where the Owner relation was already set to a different record. + $isSpecialCase = $ownerClass === WasHasOneLinkableLinkOwner::class && $ownerFixture === 'owns-has-one-again'; + // Relation should be for link, but should not have its Owner set. + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + $this->assertSame( + [ + $isSpecialCase ? $this->idFromFixture(WasHasOneLinkableLinkOwner::class, 'owns-has-one') : 0, + $isSpecialCase ? 'Link' : null + ], + [ + $link->OwnerID, + $link->OwnerRelation, + ], + "Relation {$relation} should not have an Owner set" + ); + continue; + } + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + $this->assertSame( + [ + $owner->ID, + $owner->ClassName, + $relation, + ], + [ + $link->OwnerID, + $link->OwnerClass, + $link->OwnerRelation, + ], + "Relation {$relation} should have an Owner set" + ); + } + } + } + + $this->assertSame("Setting owners for has_one relations.\n", $output); + } + private function setSortInRecord(array $record, int $sort, bool $hasSort): array { if (!$hasSort) { diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml index 29299fd8..f4d7f4fd 100644 --- a/tests/php/Tasks/GorriecoeMigrationTaskTest.yml +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest.yml @@ -117,6 +117,24 @@ GorriecoeMigrationTaskTest_OldLinkTable: Title: 'custom link03' Type: 'Custom' CustomField: 'another value' + has-one-link-1: + Title: 'HasOne Link 1' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 1 + has-one-link-2: + Title: 'HasOne Link 2' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 2 + has-one-link-3: + Title: 'HasOne Link 3' + Type: 'URL' + URL: 'http://www.silverstripe.org' + OpenInNewWindow: true + MySort: 3 SilverStripe\LinkField\Tests\Tasks\LinkFieldMigrationTaskTest\HasManyLinkOwner: # We can't add the relations here, because that would set them against a real has_one, but we want @@ -228,3 +246,34 @@ GorriecoeMigrationTaskTest_manymany_throughpoly: OldOwnerClass: 'SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasManyManyOwner' OldLinkID: =>SilverStripe\LinkField\Models\EmailLink.email-link01 CustomSort: 4 + +SilverStripe\LinkField\Models\Link: + link-empty-1: + link-empty-2: + link-empty-3: + link-empty-4: + link-empty-5: + link-empty-6: + link-1: + LinkText: 'HasOne Link 10' + OpenInNew: 1 + Sort: 10 + link-2: + LinkText: 'HasOne Link 11' + OpenInNew: 1 + Sort: 11 + link-3: + LinkText: 'HasOne Link 12' + OpenInNew: 1 + Sort: 12 + +SilverStripe\LinkField\Tests\Tasks\GorriecoeMigrationTaskTest\WasHasOneLinkableLinkOwner: + owner-1: + Title: 'HasOne Link Owner 1' + LinkID: =>SilverStripe\LinkField\Models\Link.link-1 + owner-2: + Title: 'HasOne Link Owner 2' + LinkID: =>SilverStripe\LinkField\Models\Link.link-2 + owner-3: + Title: 'HasOne Link Owner 3' + LinkID: =>SilverStripe\LinkField\Models\Link.link-3 diff --git a/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkableLinkOwner.php b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkableLinkOwner.php new file mode 100644 index 00000000..da89d5eb --- /dev/null +++ b/tests/php/Tasks/GorriecoeMigrationTaskTest/WasHasOneLinkableLinkOwner.php @@ -0,0 +1,16 @@ + Link::class, + ]; +} diff --git a/tests/php/Tasks/LinkableMigrationTask/CustomLinkableLink.php b/tests/php/Tasks/LinkableMigrationTask/CustomLinkableLink.php new file mode 100644 index 00000000..404ec1e3 --- /dev/null +++ b/tests/php/Tasks/LinkableMigrationTask/CustomLinkableLink.php @@ -0,0 +1,15 @@ + HasManyLinkableLinkOwner::class, + ]; +} diff --git a/tests/php/Tasks/LinkableMigrationTaskTest.php b/tests/php/Tasks/LinkableMigrationTaskTest.php index 681b747a..053d850d 100644 --- a/tests/php/Tasks/LinkableMigrationTaskTest.php +++ b/tests/php/Tasks/LinkableMigrationTaskTest.php @@ -60,20 +60,15 @@ public function onBeforeLoadFixtures(): void $linkDbColumns = [ ...DataObject::config()->uninherited('fixed_fields'), // Fields directly from the Link class - 'Title' => 'Varchar', - 'Type' => 'Varchar(50)', - 'URL' => 'Text', - 'Email' => 'Varchar', - 'Phone' => 'Varchar(30)', + 'Title' => 'Varchar(255)', + 'Type' => 'Varchar', + 'URL' => 'Varchar(255)', + 'Email' => 'Varchar(255)', + 'Phone' => 'Varchar(255)', 'OpenInNewWindow' => 'Boolean', - 'SelectedStyle' => 'Varchar', 'FileID' => 'ForeignKey', - // Fields from the LinkSiteTree extension 'Anchor' => 'Varchar(255)', 'SiteTreeID' => 'ForeignKey', - // Field for a custom link type - 'CustomField' => 'Varchar', - // Field for custom sort 'MySort' => 'Int', ]; DB::require_table(self::OLD_LINK_TABLE, $linkDbColumns, options: DataObject::config()->get('create_table_options')); @@ -81,20 +76,6 @@ public function onBeforeLoadFixtures(): void parent::onBeforeLoadFixtures(); } - protected function tearDown(): void - { - parent::tearDown(); - } - - public function testGetNeedsMigration() - { - $this->startCapturingOutput(); - $result = $this->callPrivateMethod('getNeedsMigration'); - $output = $this->stopCapturingOutput(); - $this->assertSame($expected, $result); - $this->assertSame($expected ? '' : "Nothing to migrate - old link table doesn't exist.\n", $output); - } - public function testInsertBaseRows(): void { // Remove existing links which can cause ID conflicts. @@ -108,10 +89,6 @@ public function testInsertBaseRows(): void $select = new SQLSelect(from: DB::get_conn()->escapeIdentifier(DataObject::getSchema()->baseDataTable(Link::class))); foreach ($select->execute() as $link) { - // Skip any links that already existed - if (str_starts_with($link['LinkText'], 'pre-existing')) { - continue; - } // The owner class is likely to be some arbitrary model - see https://github.com/silverstripe/silverstripe-framework/issues/11165 unset($link['OwnerClass']); $oldLinkSelect = new SQLSelect(from: DB::get_conn()->escapeIdentifier(self::OLD_LINK_TABLE), where: ['ID' => $link['ID']]); @@ -172,6 +149,82 @@ public function testInsertTypeSpecificRows(): void $this->assertEmpty($output); } + public function testSetOwnerForHasOneLinks(): void + { + $this->startCapturingOutput(); + $this->callPrivateMethod('setOwnerForHasOneLinks'); + $output = $this->stopCapturingOutput(); + + $ownerClass = HasOneLinkableLinkOwner::class; + $fixtureRelationsHaveLink = [ + 'owner-1' => ['Link' => true], + 'owner-2' => ['Link' => true], + 'owner-3' => ['Link' => true], + ]; + + $relationsByID = []; + foreach ($fixtureRelationsHaveLink as $fixture => $data) { + $data['fixture'] = $fixture; + $relationsByID[$this->idFromFixture($ownerClass, $fixture)] = $data; + } + + foreach (DataObject::get($ownerClass) as $owner) { + if (array_key_exists($owner->ID, $relationsByID)) { + $data = $relationsByID[$owner->ID]; + $ownerFixture = $data['fixture']; + $record = $this->objFromFixture($ownerClass, $ownerFixture); + foreach ($data as $relation => $hasLink) { + if ($relation === 'fixture') { + continue; + } + /** @var Link $link */ + $link = $record->$relation(); + if ($hasLink === null) { + // Relation should either not be for link, or should not be in the DB. + if (is_a($link->ClassName, Link::class, true)) { + $this->assertFalse($link->isInDB(), "Relation {$relation} should not have a link saved in it"); + } + continue; + } elseif ($hasLink === false) { + // The special case is where the Owner relation was already set to a different record. + $isSpecialCase = $ownerClass === HasOneLinkableLinkOwner::class && $ownerFixture === 'owns-has-one-again'; + // Relation should be for link, but should not have its Owner set. + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + // Can't check OwnerClass here - see https://github.com/silverstripe/silverstripe-framework/issues/11165 + $this->assertSame( + [ + $isSpecialCase ? $this->idFromFixture(HasOneLinkableLinkOwner::class, 'owns-has-one') : 0, + $isSpecialCase ? 'Link' : null + ], + [ + $link->OwnerID, + $link->OwnerRelation, + ], + "Relation {$relation} should not have an Owner set" + ); + continue; + } + $this->assertTrue($link->isInDB(), "Relation {$relation} should have a link saved in it"); + $this->assertSame( + [ + $owner->ID, + $owner->ClassName, + $relation, + ], + [ + $link->OwnerID, + $link->OwnerClass, + $link->OwnerRelation, + ], + "Relation {$relation} should have an Owner set" + ); + } + } + } + + $this->assertSame("Setting owners for has_one relations.\n", $output); + } + private function startCapturingOutput(): void { flush(); @@ -186,13 +239,9 @@ private function stopCapturingOutput(): string private function callPrivateMethod(string $methodName, array $args = []): mixed { $task = new LinkableMigrationTask(); - // getNeedsMigration() sets the table to pull from. - // If we're not testing that method, we need to set the table ourselves. - if ($this->getName() !== 'testGetNeedsMigration') { - $reflectionProperty = new ReflectionProperty($task, 'oldTableName'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue($task, self::OLD_LINK_TABLE); - } + $reflectionProperty = new ReflectionProperty($task, 'oldTableName'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($task, self::OLD_LINK_TABLE); $reflectionMethod = new ReflectionMethod($task, $methodName); $reflectionMethod->setAccessible(true); return $reflectionMethod->invoke($task, ...$args); diff --git a/tests/php/Tasks/LinkableMigrationTaskTest.yml b/tests/php/Tasks/LinkableMigrationTaskTest.yml index d925c157..a1a09147 100644 --- a/tests/php/Tasks/LinkableMigrationTaskTest.yml +++ b/tests/php/Tasks/LinkableMigrationTaskTest.yml @@ -71,3 +71,28 @@ LinkableMigrationTaskTest_OldLinkTable: URL: 'http://www.silverstripe.org' OpenInNewWindow: true MySort: 12 + +SilverStripe\LinkField\Models\Link: + link-1: + LinkText: 'HasOne Link 10' + OpenInNew: 1 + Sort: 10 + link-2: + LinkText: 'HasOne Link 11' + OpenInNew: 1 + Sort: 11 + link-3: + LinkText: 'HasOne Link 12' + OpenInNew: 1 + Sort: 12 + +SilverStripe\LinkField\Tests\Tasks\LinkableMigrationTaskTest\HasOneLinkableLinkOwner: + owner-1: + Title: 'HasOne Link Owner 1' + LinkID: =>SilverStripe\LinkField\Models\Link.link-1 + owner-2: + Title: 'HasOne Link Owner 2' + LinkID: =>SilverStripe\LinkField\Models\Link.link-2 + owner-3: + Title: 'HasOne Link Owner 3' + LinkID: =>SilverStripe\LinkField\Models\Link.link-3