feat(migration): add type renaming to resolve native asset name conflicts#472
feat(migration): add type renaming to resolve native asset name conflicts#472MyvTsv wants to merge 12 commits into
Conversation
|
Does this apply to names that are too long? And CLI ? |
|
CLI compatibility would be a plus. |
|
At the time of renaming, it should also be ensured that the total length (PluginGenericObject + New name) does not exceed the maximum allowed size for MySQL table names. |
Rom1-B
left a comment
Rom1-B
left a comment
There was a problem hiding this comment.
It works.
Could you add a check that alerts if the table name exceeds 64 characters?
In the CLI, add at least one check and a message to prompt renaming in the interface.
Rom1-B
left a comment
Rom1-B
left a comment
There was a problem hiding this comment.
We also need to add the case where the Fields plugin adds fields
…ed in Fields dropdowns
Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com>
|
@MyvTsv, could you please submit this fix for validation? |
| ProfileRight::cleanAllPossibleRights(); | ||
| $migration->executeMigration(); |
There was a problem hiding this comment.
ProfileRight::cleanAllPossibleRights() was called before $migration->executeMigration(), so the profile-right rename post-query (added inside updateNameAndItemtype() at line 1189) ran after the cleanup. cleanAllPossibleRights() removes the old right name (the renamed class no longer exists), and the subsequent UPDATE finds nothing to rename. Result: users lose their profile rights for any renamed type. Fixed by swapping the two lines.
| ProfileRight::cleanAllPossibleRights(); | |
| $migration->executeMigration(); | |
| $migration->executeMigration(); | |
| ProfileRight::cleanAllPossibleRights(); |
| if ($DB->fieldExists($table_name, $fkey_newname)) { | ||
| throw new RuntimeException( | ||
| sprintf( | ||
| 'Field "%s" cannot be renamed in table "%s" as "%s" is field already exists.', |
There was a problem hiding this comment.
| 'Field "%s" cannot be renamed in table "%s" as "%s" is field already exists.', | |
| 'Field "%s" cannot be renamed in table "%s": field "%s" already exists.', |
| if (str_contains($table_name, strtolower($old_itemtype))) { | ||
| $new_table_name = str_replace(strtolower($old_itemtype), strtolower($new_itemtype), $table_name); | ||
| $migration->renameTable($table_name, $new_table_name); | ||
| $table_name = $new_table_name; | ||
| } |
There was a problem hiding this comment.
str_contains($table_name, strtolower($old_itemtype)) never matches standard GLPI plugin table names: GLPI converts PluginGenericobjectCar to glpi_plugin_genericobject_cars (snake_case), so the string "plugingenericobjectcar" does not appear in any standard table name. The table-rename branch is dead code; only the itemtypes JSON-field UPDATE (lines 1312-1330) does real work.
| </div> | ||
| <small id="{{ modal_id }}-counter" | ||
| class="text-muted flex-shrink-0"> | ||
| {{ genericobject_type.name|length }}/{{ constant('PluginGenericobjectType::MAX_TYPE_NAME_LENGTH') }} |
There was a problem hiding this comment.
| {{ genericobject_type.name|length }}/{{ constant('PluginGenericobjectType::MAX_TYPE_NAME_LENGTH') }} | |
| {{ call('PluginGenericobjectType::getSystemName', [genericobject_type.name])|length }}/{{ constant('PluginGenericobjectType::MAX_TYPE_NAME_LENGTH') }} |
4 participants
Description
Screenshots (if appropriate):
Conflict:
Migrate
