Skip to content

Feat: rename oversized containers from their edit form - #1248

Open
Rom1-B wants to merge 5 commits into
mainfrom
support_45901
Open

Rom1-B wants to merge 5 commits into
mainfrom
support_45901

Conversation

@Rom1-B

@Rom1-B Rom1-B commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • It fixes !45901
  • When a container's computed table name exceeds MySQL's 64-character limit, the plugin now disables it instead of crashing, and shows a rename form directly on the container's edit page.
  • Submitting a shorter name renames the container and reactivates it; if a table with matching data already exists under that name (e.g. after a version migration), its data is preserved instead of being replaced by an empty table.
  • plugins:fields:check_database now also reports container/item type pairs with no matching table, and tables with no matching container/item type pair.

Screenshots (if appropriate):

Comment thread inc/checkdatabasecommand.class.php
Comment thread inc/checkdatabasecommand.class.php
Comment thread inc/container.class.php
@Rom1-B
Rom1-B requested a review from stonebuzz August 24, 2026 09:56
@Rom1-B
Rom1-B requested review from Otijom and removed request for Otijom September 17, 2026 12:38
$this->assertFalse($result);
}

public function testRenameOversizedContainerFailsWhenNameIsStillTooLong(): void

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And testRenameOversizedContainerFailsOnNameCollision() tests never make the container oversized, so renameOversizedContainer() returns early on "Container is not oversized". Neither the length check nor the collision check is actually exercised. Force the long name like in testRenameOversizedContainerSucceeds.

Comment thread inc/container.class.php Outdated
return false;
}

$new_name = preg_replace('/[^\da-zA-Z]/', '', $new_name) ?? '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Digits are allowed here, but the "fix container names prior to 1.9.2" step (REGEXP \d+) will rename the container again on the next update. Could we reuse PluginFieldsToolbox::getSystemNameFromLabel() instead?

Comment thread inc/container.class.php Outdated
* @param int $container_id Container ID.
* @param string[] $already_claimed Orphan tables already assigned to another itemtype in this call.
*/
private static function findOrphanTableForContainer(int $container_id, array $already_claimed): ?string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With several itemtypes, every orphan table has the same plugin_fields_containers_id default, so $by_default holds more than one entry, and the column fallback also finds several candidates. Nothing is recovered, and the message says "no existing data found". Also matching on the itemtype column default should fix it.

Comment thread inc/container.class.php
Comment on lines +253 to +256
// Disable oversized-table containers instead of crashing.
$obj = new self();
$active_containers = $obj->find(['is_active' => 1]);
foreach ($active_containers as $container) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop leaves disabled containers without a table, but preItemPurge() iterates over all containers regardless of is_active, so purging an item of that itemtype will throw a SQL error. Filter on is_active there, or skip class generation for oversized containers.

@Rom1-B
Rom1-B requested a review from RomainLvr September 25, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants