Skip to content

Commit 386f327

Browse files
christianbeeznestchristianbeeznest
authored
Merge pull request #7373 from christianbeeznest/rna-23159-3
Internal: Fast attendance migration + safe cleanup flags - refs BT#23159
2 parents 8bfbba5 + 9cbaead commit 386f327

2 files changed

Lines changed: 192 additions & 82 deletions

File tree

‎src/CoreBundle/Command/MigrateAttendancesFastCommand.php‎

Lines changed: 180 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
namespace Chamilo\CoreBundle\Command;
88

9+
use Chamilo\CoreBundle\Command\DoctrineMigrationsMigrateCommandDecorator;
910
use Doctrine\DBAL\Connection;
1011
use Doctrine\DBAL\Exception as DbalException;
1112
use Doctrine\DBAL\ParameterType;
@@ -54,6 +55,13 @@ protected function configure(): void
5455
'Alias of --drop-c-item-property (drops legacy table c_item_property after successful migration, only if no pending attendances remain).'
5556
);
5657

58+
$this->addOption(
59+
'drop-c-id-session-id-from-c-attendance',
60+
null,
61+
InputOption::VALUE_NONE,
62+
'Drop legacy columns c_attendance.c_id and c_attendance.session_id after successful migration (only if no pending attendances remain).'
63+
);
64+
5765
$this->addOption(
5866
'dry-run',
5967
null,
@@ -73,39 +81,77 @@ protected function execute(InputInterface $input, OutputInterface $output): int
7381
$dropItemProperty = (bool) $input->getOption('drop-c-item-property')
7482
|| (bool) $input->getOption('drop-c-item-properties');
7583

84+
$dropAttendanceLegacyColumns = (bool) $input->getOption('drop-c-id-session-id-from-c-attendance');
85+
7686
if ($dryRun && $dropItemProperty) {
7787
$io->note('Dry-run enabled: ignoring --drop-c-item-property (no schema changes will be applied).');
7888
$dropItemProperty = false;
7989
}
8090

91+
if ($dryRun && $dropAttendanceLegacyColumns) {
92+
$io->note('Dry-run enabled: ignoring --drop-c-id-session-id-from-c-attendance (no schema changes will be applied).');
93+
$dropAttendanceLegacyColumns = false;
94+
}
95+
8196
$fallbackAdminId = $this->getFallbackAdminId();
8297
$uuidIsBinary = $this->detectUuidIsBinary();
8398

8499
$hasItemProperty = $this->tableExists('c_item_property');
100+
101+
// We rely on c_attendance.c_id to map attendances to courses (c_item_property.c_id is ignored).
85102
$hasAttendanceCId = $this->tableHasColumn('c_attendance', 'c_id');
103+
$hasAttendanceLegacyId = $this->tableHasColumn('c_attendance', 'id');
104+
$hasAttendanceSessionId = $this->tableHasColumn('c_attendance', 'session_id');
105+
86106
$hasAttendanceTitle = $this->tableHasColumn('c_attendance', 'title');
87107
$hasAttendanceName = $this->tableHasColumn('c_attendance', 'name');
88108

89-
// At this migration stage, c_attendance.session_id is expected to be removed.
90-
// We only rely on c_item_property.session_id when available.
91-
$hasItemPropertySessionId = $hasItemProperty && $this->tableHasColumn('c_item_property', 'session_id');
92-
93-
if (!$hasItemProperty && !$hasAttendanceCId) {
94-
$io->error('Cannot determine attendance->course mapping: c_item_property does not exist and c_attendance.c_id does not exist.');
109+
if (!$hasAttendanceCId) {
110+
$io->error('Cannot map attendances to courses: c_attendance.c_id is missing. This command expects c_id to be available in c_attendance.');
95111
return Command::FAILURE;
96112
}
97113

98-
if ($hasItemProperty && !$hasItemPropertySessionId) {
99-
$io->note('c_item_property.session_id is not available. Session context will be stored as NULL in resource_link.');
114+
if (!$hasAttendanceSessionId) {
115+
$io->note('c_attendance.session_id is not available. Session context will be stored as NULL in resource_link.');
116+
}
117+
118+
// Respect the same env-flag used during Doctrine migrations (only migrate gradebook-linked attendances).
119+
$skipAttendances = (bool) getenv(DoctrineMigrationsMigrateCommandDecorator::SKIP_ATTENDANCES_FLAG);
120+
$gradebookIds = [];
121+
122+
if ($skipAttendances) {
123+
$io->note('SKIP_ATTENDANCES flag detected: only gradebook-linked attendances will be migrated.');
124+
125+
// gradebook_link.type=7 (attendance). Some datasets may link to attendance.iid or attendance.id.
126+
$join = 'a.iid = gl.ref_id';
127+
if ($hasAttendanceLegacyId) {
128+
$join = '(a.iid = gl.ref_id OR a.id = gl.ref_id)';
129+
}
130+
131+
$ids = $this->connection->fetchFirstColumn(
132+
"SELECT DISTINCT a.iid
133+
FROM gradebook_link gl
134+
INNER JOIN c_attendance a ON {$join}
135+
WHERE gl.type = 7"
136+
);
137+
138+
$ids = array_map('intval', $ids);
139+
$gradebookIds = array_fill_keys($ids, true);
100140
}
101141

102-
$courseIds = $this->getCourseIdsToProcess($hasItemProperty, $hasAttendanceCId);
142+
$courseIds = $this->getCourseIdsToProcess();
103143

104144
if (0 === \count($courseIds)) {
105145
$io->success('No attendances to migrate (nothing pending).');
146+
106147
if ($dropItemProperty) {
107148
$this->maybeDropItemProperty($io);
108149
}
150+
151+
if ($dropAttendanceLegacyColumns) {
152+
$this->maybeDropAttendanceLegacyColumns($io);
153+
}
154+
109155
return Command::SUCCESS;
110156
}
111157

@@ -149,11 +195,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
149195

150196
$attendanceRows = $this->fetchPendingAttendancesForCourse(
151197
courseId: $courseId,
152-
hasItemProperty: $hasItemProperty,
153-
hasAttendanceCId: $hasAttendanceCId,
154198
hasAttendanceTitle: $hasAttendanceTitle,
155199
hasAttendanceName: $hasAttendanceName,
156-
hasItemPropertySessionId: $hasItemPropertySessionId
200+
hasAttendanceSessionId: $hasAttendanceSessionId,
201+
hasAttendanceLegacyId: $hasAttendanceLegacyId
157202
);
158203

159204
if (0 === \count($attendanceRows)) {
@@ -168,22 +213,52 @@ protected function execute(InputInterface $input, OutputInterface $output): int
168213
try {
169214
foreach ($attendanceRows as $row) {
170215
$attendanceId = (int) $row['iid'];
216+
217+
if ($skipAttendances && !isset($gradebookIds[$attendanceId])) {
218+
continue;
219+
}
220+
171221
$attendanceTitle = $this->pickAttendanceTitle($row, $attendanceId);
172222

173-
// session_id is read from c_item_property (when available).
174-
// Normalize 0 -> NULL as expected by resource_link.session_id.
175-
$attendanceSessionId = isset($row['session_id']) ? (int) $row['session_id'] : 0;
176-
$attendanceSessionId = 0 === $attendanceSessionId ? null : $attendanceSessionId;
223+
$attendanceLegacyId = null;
224+
if ($hasAttendanceLegacyId && isset($row['legacy_id']) && null !== $row['legacy_id']) {
225+
$legacy = (int) $row['legacy_id'];
226+
$attendanceLegacyId = $legacy > 0 ? $legacy : null;
227+
}
228+
229+
// IMPORTANT:
230+
// - We ignore c_item_property.session_id because it can be incoherent.
231+
// - We store session context using c_attendance.session_id (when available).
232+
$attendanceSessionId = null;
233+
if ($hasAttendanceSessionId && isset($row['attendance_session_id']) && null !== $row['attendance_session_id']) {
234+
$tmp = (int) $row['attendance_session_id'];
235+
$attendanceSessionId = $tmp > 0 ? $tmp : null;
236+
}
177237

238+
// Metadata from c_item_property:
239+
// - Trust only tool + ref (and optionally legacy ref=a.id).
240+
// - Do NOT filter by c_id to avoid relying on incoherent mappings.
178241
$ip = [];
179242
if ($hasItemProperty) {
180-
$ip = $this->connection->fetchAssociative(
181-
"SELECT insert_date, lastedit_date, lastedit_user_id, visibility, start_visible, end_visible, to_group_id, to_user_id
182-
FROM c_item_property
183-
WHERE tool = 'attendance' AND ref = :ref AND c_id = :cid
184-
LIMIT 1",
185-
['ref' => $attendanceId, 'cid' => $courseId]
186-
) ?: [];
243+
$sql = "SELECT insert_date, lastedit_date, lastedit_user_id, visibility, start_visible, end_visible, to_group_id, to_user_id
244+
FROM c_item_property
245+
WHERE tool = 'attendance' AND ref = :iid
246+
ORDER BY insert_date ASC
247+
LIMIT 1";
248+
249+
$params = ['iid' => $attendanceId];
250+
251+
if (null !== $attendanceLegacyId) {
252+
$sql = "SELECT insert_date, lastedit_date, lastedit_user_id, visibility, start_visible, end_visible, to_group_id, to_user_id
253+
FROM c_item_property
254+
WHERE tool = 'attendance'
255+
AND (ref = :iid OR ref = :legacyId)
256+
ORDER BY CASE WHEN ref = :iid THEN 1 ELSE 0 END DESC, insert_date ASC
257+
LIMIT 1";
258+
$params['legacyId'] = $attendanceLegacyId;
259+
}
260+
261+
$ip = $this->connection->fetchAssociative($sql, $params) ?: [];
187262
}
188263

189264
$insertDate = $ip['insert_date'] ?? $this->nowUtc();
@@ -236,7 +311,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
236311
'user_id' => $toUserId,
237312
]);
238313

239-
// resource_node.path should follow the same structure as the standard migration:
314+
// resource_node.path format:
240315
// <parentPath>/<title>-<attendanceIid>-<nodeId>/
241316
$segmentTitle = trim(str_replace(['/', '\\'], '-', $attendanceTitle));
242317
$segmentTitle = preg_replace('/\s+/', ' ', $segmentTitle) ?: $segmentTitle;
@@ -291,74 +366,49 @@ protected function execute(InputInterface $input, OutputInterface $output): int
291366
}
292367
}
293368

369+
if ($dropAttendanceLegacyColumns) {
370+
$this->maybeDropAttendanceLegacyColumns($io);
371+
} else {
372+
if ($this->tableHasColumn('c_attendance', 'c_id') || $this->tableHasColumn('c_attendance', 'session_id')) {
373+
$io->note('c_attendance legacy columns still exist. You can drop them later or rerun this command with --drop-c-id-session-id-from-c-attendance once you confirm no pending attendances remain.');
374+
}
375+
}
376+
294377
return Command::SUCCESS;
295378
}
296379

297-
private function getCourseIdsToProcess(bool $hasItemProperty, bool $hasAttendanceCId): array
380+
private function getCourseIdsToProcess(): array
298381
{
299-
if ($hasItemProperty) {
300-
return $this->connection->fetchFirstColumn(
301-
"SELECT DISTINCT c_id
302-
FROM c_item_property
303-
WHERE tool = 'attendance'
304-
ORDER BY c_id"
305-
);
306-
}
307-
308-
// Fallback: legacy schema still has c_attendance.c_id
309-
if ($hasAttendanceCId) {
310-
return $this->connection->fetchFirstColumn(
311-
"SELECT DISTINCT c_id
312-
FROM c_attendance
313-
WHERE resource_node_id IS NULL
314-
ORDER BY c_id"
315-
);
316-
}
317-
318-
return [];
382+
// We rely on c_attendance.c_id to identify the course ownership.
383+
return $this->connection->fetchFirstColumn(
384+
"SELECT DISTINCT c_id
385+
FROM c_attendance
386+
WHERE resource_node_id IS NULL
387+
AND c_id IS NOT NULL
388+
ORDER BY c_id"
389+
);
319390
}
320391

321392
private function fetchPendingAttendancesForCourse(
322393
int $courseId,
323-
bool $hasItemProperty,
324-
bool $hasAttendanceCId,
325394
bool $hasAttendanceTitle,
326395
bool $hasAttendanceName,
327-
bool $hasItemPropertySessionId
396+
bool $hasAttendanceSessionId,
397+
bool $hasAttendanceLegacyId
328398
): array {
329399
$selectTitle = $hasAttendanceTitle ? 'a.title' : 'NULL AS title';
330400
$selectName = $hasAttendanceName ? 'a.name' : 'NULL AS name';
331-
332-
// session_id comes ONLY from c_item_property when available, otherwise NULL.
333-
$selectSession = $hasItemPropertySessionId ? 'ip.session_id' : 'NULL';
334-
335-
if ($hasItemProperty) {
336-
return $this->connection->fetchAllAssociative(
337-
"SELECT a.iid, {$selectTitle}, {$selectName}, {$selectSession} AS session_id
338-
FROM c_attendance a
339-
INNER JOIN c_item_property ip
340-
ON ip.tool = 'attendance'
341-
AND ip.ref = a.iid
342-
AND ip.c_id = :cid
343-
WHERE a.resource_node_id IS NULL
344-
ORDER BY a.iid",
345-
['cid' => $courseId]
346-
);
347-
}
348-
349-
// Fallback using legacy c_id (no c_item_property available).
350-
// At this stage, we cannot infer a session_id, so we store NULL.
351-
if ($hasAttendanceCId) {
352-
return $this->connection->fetchAllAssociative(
353-
"SELECT a.iid, {$selectTitle}, {$selectName}, NULL AS session_id
354-
FROM c_attendance a
355-
WHERE a.c_id = :cid AND a.resource_node_id IS NULL
356-
ORDER BY a.iid",
357-
['cid' => $courseId]
358-
);
359-
}
360-
361-
return [];
401+
$selectSession = $hasAttendanceSessionId ? 'a.session_id AS attendance_session_id' : 'NULL AS attendance_session_id';
402+
$selectLegacyId = $hasAttendanceLegacyId ? 'a.id AS legacy_id' : 'NULL AS legacy_id';
403+
404+
return $this->connection->fetchAllAssociative(
405+
"SELECT a.iid, {$selectTitle}, {$selectName}, {$selectSession}, {$selectLegacyId}
406+
FROM c_attendance a
407+
WHERE a.c_id = :cid
408+
AND a.resource_node_id IS NULL
409+
ORDER BY a.iid",
410+
['cid' => $courseId]
411+
);
362412
}
363413

364414
private function pickAttendanceTitle(array $row, int $attendanceId): string
@@ -405,6 +455,57 @@ private function maybeDropItemProperty(SymfonyStyle $io): void
405455
}
406456
}
407457

458+
/**
459+
* Drops legacy columns from c_attendance.
460+
* Only runs if no pending attendances remain.
461+
*/
462+
private function maybeDropAttendanceLegacyColumns(SymfonyStyle $io): void
463+
{
464+
if (!$this->tableExists('c_attendance')) {
465+
$io->note('Table "c_attendance" does not exist - nothing to drop.');
466+
return;
467+
}
468+
469+
$pending = (int) $this->connection->fetchOne('SELECT COUNT(*) FROM c_attendance WHERE resource_node_id IS NULL');
470+
if ($pending > 0) {
471+
$io->warning("Not dropping legacy columns from c_attendance: {$pending} attendances are still pending (resource_node_id IS NULL).");
472+
return;
473+
}
474+
475+
$sm = $this->connection->createSchemaManager();
476+
$table = $sm->introspectTable('c_attendance');
477+
478+
$columnsToDrop = ['c_id', 'session_id'];
479+
$dropList = [];
480+
481+
foreach ($columnsToDrop as $col) {
482+
if ($table->hasColumn($col)) {
483+
$dropList[] = $col;
484+
}
485+
}
486+
487+
if (0 === \count($dropList)) {
488+
$io->note('c_attendance does not have legacy columns c_id/session_id - nothing to drop.');
489+
return;
490+
}
491+
492+
$io->section('Dropping legacy columns from c_attendance...');
493+
494+
try {
495+
foreach ($dropList as $col) {
496+
// Keep it explicit to avoid relying on non-portable "IF EXISTS" syntax.
497+
$this->connection->executeStatement("ALTER TABLE c_attendance DROP COLUMN {$col}");
498+
$io->writeln(" - Dropped column c_attendance.{$col}");
499+
}
500+
501+
$io->success('Legacy columns dropped from c_attendance.');
502+
} catch (DbalException $e) {
503+
$io->error('Failed to drop legacy columns from c_attendance: '.$e->getMessage());
504+
} catch (\Throwable $e) {
505+
$io->error('Failed to drop legacy columns from c_attendance: '.$e->getMessage());
506+
}
507+
}
508+
408509
private function getFallbackAdminId(): int
409510
{
410511
$id = $this->connection->fetchOne(

‎src/CoreBundle/Migrations/Schema/V200/Version20240811221400.php‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Chamilo\CoreBundle\Migrations\Schema\V200;
66

7+
use Chamilo\CoreBundle\Command\DoctrineMigrationsMigrateCommandDecorator;
78
use Chamilo\CoreBundle\Migrations\AbstractMigrationChamilo;
89
use Doctrine\DBAL\Schema\Schema;
910

@@ -16,6 +17,10 @@ public function getDescription(): string
1617

1718
public function up(Schema $schema): void
1819
{
20+
// When enabled, we keep legacy attendance columns to avoid data loss
21+
// if attendances are being skipped/handled separately.
22+
$skipAttendances = (bool) getenv(DoctrineMigrationsMigrateCommandDecorator::SKIP_ATTENDANCES_FLAG);
23+
1924
$this->addSql('SET FOREIGN_KEY_CHECKS = 0;');
2025

2126
// resource_node
@@ -213,9 +218,13 @@ public function up(Schema $schema): void
213218
}
214219

215220
// c_attendance
216-
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS c_id');
217-
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS id');
218-
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS session_id');
221+
if ($skipAttendances) {
222+
$this->write('Skip attendances flag enabled: keeping legacy c_attendance columns (c_id, id, session_id).');
223+
} else {
224+
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS c_id');
225+
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS id');
226+
$this->addSql('ALTER TABLE c_attendance DROP COLUMN IF EXISTS session_id');
227+
}
219228

220229
// c_forum_thread
221230
$this->addSql('ALTER TABLE c_forum_thread DROP FOREIGN KEY IF EXISTS FK_5DA7884CD4DC43B9');

0 commit comments

Comments
 (0)