refactor(tenant): give every table one spelling of the tenant pair

Phase 3 of the tenant-marking series. The same concept was written four ways,
and the Doctrine filter arriving in phase 4 keys on the field name — so the
tables using a different spelling would have been skipped silently, which is
exactly the leak this work exists to prevent.

- discount_rules: owner_type/owner_id renamed to entity_type/entity_id. Pure
  rename, no data moves.
- doctor_secretaries: owner_type plus a nullable clinic_id replaced by the
  shared pair. The environment now comes from the clinic argument alone, so the
  inconsistent combination (owner_type='clinic', clinic_id=NULL) can no longer
  be constructed, and the redundant constructor parameter is gone.
- user_active_context: added db_type, so resolving an environment is one lookup
  instead of "try clinics, then try doctors". Filled from the type already
  present in available_contexts.
- entity_type is VARCHAR(10) in all twenty tenant tables; four of them were 20.

Behaviour change, the only one in this series: the doctor_secretaries unique key
went from (doctor_id, secretary_id, owner_type) to (doctor_id, secretary_id,
entity_type, entity_id). With clinic_id outside the key, one secretary could not
be assigned to the same doctor in two clinics — the second row collided on
owner_type='clinic'. The duplicate check in SecretaryController had the same
blind spot and would have rejected the request before the database saw it; both
are fixed together.

Correcting an assumption from the phase-3 plan: mobile_verification_otp.entity_type
really is a tenant pair. NotificationMobileController validates the target against
['doctor','clinic'] and stores that entity's id, so the column was normalised with
the rest rather than treated as unrelated.

TenantOwnedTrait gained assignTenantPair() for callers that resolved the pair as
scalars and hold no entity — building an EntityContext from scalars would produce
one where isClinic() is true but ->clinic is null, breaking consumers silently.

tests/ApiTestCase::createUser now retries on a duplicate mobile. db_test is never
reset and already holds ~38k users, so the 9-digit random draw collided often
enough to fail unrelated tests a few percent of runs.

Tests: 830 passing. PHPStan reports no new errors on the changed files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
hamed
2026-07-28 11:56:57 +03:30
co-authored by Claude Opus 5
parent d53874ff50
commit 2e0888e0ef
33 changed files with 728 additions and 120 deletions
+38
View File
@@ -0,0 +1,38 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20260728080123 extends AbstractMigration
{
/**
* Phase 3 of the tenant-marking series: discount_rules spelled the tenant as
* owner_type/owner_id, which the Doctrine filter added in phase 4 would not
* recognise. Pure rename — no data moves, so CHANGE is enough.
*/
public function getDescription(): string
{
return 'Rename discount_rules owner_type/owner_id to the shared entity_type/entity_id pair';
}
public function up(Schema $schema): void
{
$this->addSql('DROP INDEX idx_discount_rules_owner ON discount_rules');
$this->addSql('ALTER TABLE discount_rules CHANGE owner_type entity_type VARCHAR(10) NOT NULL, CHANGE owner_id entity_id INT NOT NULL');
$this->addSql('CREATE INDEX idx_discount_rules_tenant ON discount_rules (entity_type, entity_id, active)');
}
public function down(Schema $schema): void
{
$this->addSql('DROP INDEX idx_discount_rules_tenant ON discount_rules');
$this->addSql('ALTER TABLE discount_rules CHANGE entity_type owner_type VARCHAR(10) NOT NULL, CHANGE entity_id owner_id INT NOT NULL');
$this->addSql('CREATE INDEX idx_discount_rules_owner ON discount_rules (owner_type, owner_id, active)');
}
}
+82
View File
@@ -0,0 +1,82 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Phase 3 of the tenant-marking series: doctor_secretaries kept the tenant as
* owner_type plus a nullable clinic_id, and its unique key covered only
* (doctor_id, secretary_id, owner_type). With clinic_id outside the key, the
* same secretary could not be assigned to the same doctor in two clinics — the
* second row collided on owner_type = 'clinic'. Folding the pair into the key
* fixes that, and is the one deliberate behaviour change in this series.
*
* Statements run through $this->connection so the guards can sit between the
* backfill and the NOT NULL change; addSql() would defer them to the end.
*/
final class Version20260728080516 extends AbstractMigration
{
public function getDescription(): string
{
return 'Replace doctor_secretaries.owner_type with the shared tenant pair and widen its unique key';
}
public function up(Schema $schema): void
{
$this->connection->executeStatement(
'ALTER TABLE doctor_secretaries ADD entity_type VARCHAR(10) NULL, ADD entity_id INT NULL'
);
// A row claiming the clinic environment without a clinic is data we cannot
// interpret; stop and let a human decide rather than guessing an owner.
$broken = $this->connection->fetchFirstColumn(
"SELECT id FROM doctor_secretaries WHERE owner_type = 'clinic' AND clinic_id IS NULL"
);
$this->abortIf(
$broken !== [],
'Inconsistent doctor_secretaries rows (owner_type=clinic, clinic_id NULL): ' . implode(',', $broken)
);
$this->connection->executeStatement(
"UPDATE doctor_secretaries
SET entity_type = owner_type,
entity_id = IF(owner_type = 'clinic', clinic_id, doctor_id)"
);
$remaining = (int) $this->connection->fetchOne(
'SELECT COUNT(*) FROM doctor_secretaries WHERE entity_type IS NULL OR entity_id IS NULL'
);
$this->abortIf($remaining > 0, "Backfill left {$remaining} doctor_secretaries rows without a tenant.");
$this->connection->executeStatement(
'ALTER TABLE doctor_secretaries MODIFY entity_type VARCHAR(10) NOT NULL, MODIFY entity_id INT NOT NULL'
);
// Replacement key first, so the table is never without uniqueness cover.
$this->connection->executeStatement(
'CREATE UNIQUE INDEX uniq_doctor_secretary_scope
ON doctor_secretaries (doctor_id, secretary_id, entity_type, entity_id)'
);
$this->connection->executeStatement('DROP INDEX idx_doctor_secretary_scope ON doctor_secretaries');
$this->connection->executeStatement('ALTER TABLE doctor_secretaries DROP owner_type');
}
public function down(Schema $schema): void
{
$this->addSql("ALTER TABLE doctor_secretaries ADD owner_type VARCHAR(10) DEFAULT 'doctor' NOT NULL");
$this->addSql('UPDATE doctor_secretaries SET owner_type = entity_type');
$this->addSql('CREATE UNIQUE INDEX idx_doctor_secretary_scope ON doctor_secretaries (doctor_id, secretary_id, owner_type)');
$this->addSql('DROP INDEX uniq_doctor_secretary_scope ON doctor_secretaries');
$this->addSql('ALTER TABLE doctor_secretaries DROP entity_type, DROP entity_id');
}
/** DDL on MariaDB commits implicitly; wrapping up() in a transaction would be a lie. */
public function isTransactional(): bool
{
return false;
}
}
+64
View File
@@ -0,0 +1,64 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Phase 3 of the tenant-marking series: user_active_context stored only a
* db_uuid, which forced every environment lookup to try clinics first and fall
* back to doctors. db_type records which of the two the uuid addresses.
*
* Backfilled by matching the uuid against both tables. Rows whose uuid no longer
* resolves are deleted rather than guessed: this table is a cache of "the last
* environment the user picked", and losing a row simply sends the resolver back
* to its role fallback — the same state a brand-new user is in. This is the only
* migration in the series allowed to delete rows.
*/
final class Version20260728080955 extends AbstractMigration
{
public function getDescription(): string
{
return 'Record whether user_active_context.db_uuid points at a doctor or a clinic';
}
public function up(Schema $schema): void
{
$this->connection->executeStatement(
'ALTER TABLE user_active_context ADD db_type VARCHAR(10) NULL'
);
$this->connection->executeStatement(
"UPDATE user_active_context uac JOIN clinics c ON c.uuid = uac.db_uuid SET uac.db_type = 'clinic'"
);
$this->connection->executeStatement(
"UPDATE user_active_context uac JOIN doctors d ON d.uuid = uac.db_uuid
SET uac.db_type = 'doctor' WHERE uac.db_type IS NULL"
);
$orphans = (int) $this->connection->executeStatement(
'DELETE FROM user_active_context WHERE db_type IS NULL'
);
if ($orphans > 0) {
$this->write("Dropped {$orphans} active-context rows whose db_uuid no longer resolves.");
}
$this->connection->executeStatement(
'ALTER TABLE user_active_context MODIFY db_type VARCHAR(10) NOT NULL'
);
}
public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE user_active_context DROP db_type');
}
/** DDL on MariaDB commits implicitly; wrapping up() in a transaction would be a lie. */
public function isTransactional(): bool
{
return false;
}
}
+41
View File
@@ -0,0 +1,41 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20260728081650 extends AbstractMigration
{
/**
* Phase 3 of the tenant-marking series: four tables spelled entity_type as
* VARCHAR(20) while the other thirteen used VARCHAR(10). Only 'doctor' and
* 'clinic' are ever stored, and a mismatched width makes joins between tenant
* tables fall back to a collation conversion.
*/
public function getDescription(): string
{
return 'Give every entity_type column the same width as the rest of the tenant tables';
}
public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE inventory_items CHANGE entity_type entity_type VARCHAR(10) NOT NULL');
$this->addSql('ALTER TABLE inventory_packages CHANGE entity_type entity_type VARCHAR(10) NOT NULL');
$this->addSql('ALTER TABLE mobile_verification_otp CHANGE entity_type entity_type VARCHAR(10) NOT NULL');
$this->addSql('ALTER TABLE tenant_tags CHANGE entity_type entity_type VARCHAR(10) NOT NULL');
}
public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE inventory_items CHANGE entity_type entity_type VARCHAR(20) NOT NULL');
$this->addSql('ALTER TABLE inventory_packages CHANGE entity_type entity_type VARCHAR(20) NOT NULL');
$this->addSql('ALTER TABLE mobile_verification_otp CHANGE entity_type entity_type VARCHAR(20) NOT NULL');
$this->addSql('ALTER TABLE tenant_tags CHANGE entity_type entity_type VARCHAR(20) NOT NULL');
}
}