Any reason for adding these empty lines?

Map OnCopyData event to edit permission.

Move this to the CheckPermission method.

Move OnCopyData event content to a dedicated class with self-explanatory method names.

Missing licensing DocBlock and die statement on missing FULL_PATH.

Add $target_theme_id parameter here too.

If source and target themes match, then throw an LogicException.

1. remove parameter in favor of constructor parameter with same name
2. rename method to just copy

remove in favor of check in constructor

remove, because there can't be a case, where page/category doesn't have a revision

missing space after =

rename $revision to $revision_data (use Shift+F6 in PhpStorm)

add blank line after this one

use doUpdate method for this

add a space after RevisionId =

rename $block to $block_data

Format field list the way, that each field would be on it's own line:

based on condition in HAVING clause you want to get only matching records, so just:

1. replace LEFT JOIN with JOIN
2. remove the HAVING part

1. add blank line after this line
2. if no categories are found, then don't do implode and just exit from the method

add blank line after this line

1. remove (no need to have these in 5.3)
2. rebuild class map using in-portal classmap:rebuild command
3. be sure to include rebuild class map in the patch

remove, because we're already checking permissions thanks to code in mapPermissions method

replace with:

$ids = $this->StoreSelectedIDs($event);

1. use makeClass method call to instantiate new class
2. create object inside foreach because after other changes the constructor accepts source & destination themes

1. extract GetVar result into tmp variable
2. sanitize input to be in proper format:
   • explode by ,
   • do intval on every element (via array_map)
   • then do array_filter on a result
3. if we end up with empty ids list, then apply FALSE as filter value

None of licensing comments in this project have this part (the description).

This should be just * @version $Id$ because actual value is assigned at commit time only.

The comment should be broken (wrong * position) as other comments in the project to be consistent with other code. Later on (we have task for that) we'll fix all the comments at the same time.

The 2009 year stands for year, when file was added. It should be 2015 instead.

Remove this because __construct method doesn't have a return value.

Change exception message to When source and target themes match.

Usage of request variable not sanitized.

Please also apply filter by Type column (real templates only) to both source and target to only retrieve categories matching to TPL files. Without this categories for virtual templates (e.g. using shared design) are also retrieved, but their content blocks are shared between themes anyway.

Please also apply Type column filter (real templates only) so that we don't delete more content blocks that we're copying.

CS

wrong indentation (SPACEs used instead/in combination with of TABs)

CS

wrong indentation (SPACEs used instead/in combination with of TABs)

Replace with <inp2:m_IfDataExists>.

Reduces copy/paste of internal not_matched_theme_ids request parameter.

CS

wrong indentation (SPACEs used instead/in combination with of TABs)

1. rename to not_matched_theme_element
2. move above <inp2:m_RenderElement design="form_message">

1. block name is too general and might overwrite other block (if any) with same name
2. blocks are always declared, even when placed inside IF that is never executed

Replace with </inp2:m_IfDataExists>.

Reduces copy/paste of internal not_matched_theme_ids request parameter.

Rename into $pageContentTableName (via Shift+F6 shortcut of PhpStorm).

Other properties (e.g. categoriesTableName and pageRevisionTableName) are named according to table name they represent, but not this one.

Add a dot (.) at the end of exception message.

CS: Each exception message must end with a dot.

Move this to deleteTargetBlocks method.

In deleteTargetBlocks method all revisions are deleted so it would be logical to reset live revision number of a category as well.

Move this line up to be on JOIN line without risking to violate max line length constain.

Use PAGE_TYPE_TEMPLATE constant instead of 2.

Use named constants, when available instead of magic numbers.

We should only delete content for matched templates (found by getMatchingTemplates method) in target theme.

CMS Content of templates in target theme, that have no match in source theme would be deleted as well.

P.S.
Test plan needs to be updated to reflect this scenario.

Use PAGE_TYPE_TEMPLATE constant instead of 2.

Use named constants, when available instead of magic numbers.