feat(ccli): add CcliImportService for song upsert
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
parent
e4e5df912e
commit
091e00f255
10
.sisyphus/evidence/task-7-duplicate-throws.txt
Normal file
10
.sisyphus/evidence/task-7-duplicate-throws.txt
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
Command: ddev exec php artisan test --filter='blocks active duplicate ccli_id with DuplicateCcliSongException'
|
||||
|
||||
PASS: Active duplicate CCLI-ID is blocked.
|
||||
|
||||
Verified assertions:
|
||||
- first import succeeds
|
||||
- second import throws App\Exceptions\DuplicateCcliSongException
|
||||
- exception existingSongId matches original Song ID
|
||||
- exception message contains "existiert bereits"
|
||||
- Song::count() remains 1
|
||||
13
.sisyphus/evidence/task-7-en-only-import.txt
Normal file
13
.sisyphus/evidence/task-7-en-only-import.txt
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
Command: ddev exec php artisan test --filter='imports english-only fixture and creates song with default arrangement'
|
||||
|
||||
PASS: CcliImportService imported tests/fixtures/ccli/english-only-multi-verse.txt
|
||||
|
||||
Verified assertions:
|
||||
- status = created
|
||||
- Song title = Test Song 1
|
||||
- Song ccli_id = 9999001
|
||||
- imported_from_ccli_at is set
|
||||
- ccli_source_url = https://songselect.ccli.com/Songs/9999001
|
||||
- default arrangement name = normal, is_default = true
|
||||
- arrangement label entries = 5
|
||||
- slide rows = 9
|
||||
9
.sisyphus/evidence/task-7-restore.txt
Normal file
9
.sisyphus/evidence/task-7-restore.txt
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
Command: ddev exec php artisan test --filter='restores soft-deleted song and does not duplicate normal arrangement'
|
||||
|
||||
PASS: Soft-deleted CCLI match is restored instead of duplicated.
|
||||
|
||||
Verified assertions:
|
||||
- restored import status = restored
|
||||
- returned Song ID matches original soft-deleted Song ID
|
||||
- restored song is no longer trashed
|
||||
- only one normal arrangement exists after restore/import
|
||||
9
.sisyphus/evidence/task-7-rollback.txt
Normal file
9
.sisyphus/evidence/task-7-rollback.txt
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
Command: ddev exec php artisan test --filter='rolls back song and log when slide creation fails'
|
||||
|
||||
PASS: Transaction rollback verified via SQLite trigger failure on song_slides insert.
|
||||
|
||||
Verified assertions:
|
||||
- import throws Illuminate\Database\QueryException
|
||||
- Song::count() = 0 after failure
|
||||
- SongSlide::count() = 0 after failure
|
||||
- ApiRequestLog::count() = 0 after failure
|
||||
31
.sisyphus/notepads/ccli-songselect-import/issues.md
Normal file
31
.sisyphus/notepads/ccli-songselect-import/issues.md
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
# CCLI SongSelect Import — Issues
|
||||
|
||||
## [2026-05-10] Known Issues / Gotchas
|
||||
|
||||
### T1: Fixture corpus requires structurally accurate CCLI format
|
||||
- Plan originally asked for "real CCLI text pastes" but since we can't access SongSelect, agent creates synthetic fixtures.
|
||||
- Synthetic fixtures MUST match exact CCLI format: title line, blank, section label, lyrics, blank, footer with © and CCLI #.
|
||||
- The parser tests depend on these fixtures — structural accuracy is critical.
|
||||
|
||||
### T7: ProImportService method name
|
||||
- Plan was corrected: the public method is `ProImportService::import(UploadedFile $file)`, not `upsertSong()`.
|
||||
- When mirroring the pattern, READ app/Services/ProImportService.php before implementing.
|
||||
|
||||
### No Admin Role
|
||||
- No `admin` role or Policy exists in the codebase.
|
||||
- CCLI Settings section is visible to ALL authenticated users.
|
||||
- Document this decision, don't create a policy gate.
|
||||
|
||||
### AGENDA_KEYS whitelist
|
||||
- SettingsController has a `const AGENDA_KEYS` array.
|
||||
- T4 MUST add `'default_translation_language'` to this array OR update the validation to include it.
|
||||
- Failure to update AGENDA_KEYS = PATCH /settings will silently ignore the new key.
|
||||
|
||||
### Translation Pairing Label Direction
|
||||
- CCLI paste can have English labels; local songs may have German labels (Strophe, Refrain).
|
||||
- CcliLabels::normalizeLabelName() normalizes BOTH directions to canonical English before pairing.
|
||||
- Do NOT assume same language on both sides.
|
||||
|
||||
### T7: Global Label Slide Replacement Caveat
|
||||
- The requested CCLI import pattern deletes `songSlides()` on the resolved global `Label` before recreating slides.
|
||||
- Because labels are shared globally, a later import using the same canonical label name replaces that label's slide text globally; this intentionally matches the task spec and existing `ProImportService` pattern, but remains a design caveat for future per-song slide ownership work.
|
||||
|
|
@ -85,6 +85,12 @@ ### 2026-05-10 CcliPasteParser Implementation
|
|||
- EN/DE side-by-side imports merge only adjacent labels with different raw kinds but the same `CcliLabels::normalizeLabelName()` canonical kind/number, preserving German lyrics in `linesTranslated`.
|
||||
- DDEV/Linux path is `tests/fixtures/ccli` (lowercase); macOS accepted `tests/Fixtures/ccli`, but tests must use lowercase for container portability.
|
||||
|
||||
### 2026-05-10 CcliImportService Implementation
|
||||
- `CcliImportService` mirrors `ProImportService` by wrapping song metadata, global label resolution, slide replacement, default arrangement upsert, arrangement-label recreation, and `ApiRequestLog` success entry in one `DB::transaction()`.
|
||||
- Active duplicate CCLI IDs are blocked with `DuplicateCcliSongException`; trashed matches are restored and updated in-place via `Song::withTrashed()->where('ccli_id', ...)`.
|
||||
- CCLI label names should be canonicalized with `CcliLabels::normalizeLabelName($kind.' '.$number)` before `Label::firstOrCreate()`, keeping labels global/shared.
|
||||
- Import tests can verify rollback deterministically with a temporary SQLite trigger that aborts `song_slides` insert; this proves song + log rows are not persisted after mid-transaction failure.
|
||||
|
||||
### 2026-05-10 CCLI Parser Review Fixes
|
||||
- CCLI SongSelect metadata can appear as `CCLI Song #`, `CCLI-Nr.` or `CCLI-Liednummer`; extraction must ignore `CCLI License/Lizenz` numbers.
|
||||
- Parsed section `kind` is canonicalized via `CcliLabels::normalizeLabelName()`, while the original pasted label remains available in `label`; translation pairing still compares raw label kinds internally.
|
||||
|
|
|
|||
15
app/Exceptions/DuplicateCcliSongException.php
Normal file
15
app/Exceptions/DuplicateCcliSongException.php
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
<?php
|
||||
|
||||
namespace App\Exceptions;
|
||||
|
||||
use RuntimeException;
|
||||
|
||||
final class DuplicateCcliSongException extends RuntimeException
|
||||
{
|
||||
public function __construct(
|
||||
public readonly int $existingSongId,
|
||||
string $message = '',
|
||||
) {
|
||||
parent::__construct($message ?: "Song mit dieser CCLI-Nummer existiert bereits (#{$existingSongId})");
|
||||
}
|
||||
}
|
||||
151
app/Services/CcliImportService.php
Normal file
151
app/Services/CcliImportService.php
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
<?php
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
use App\Exceptions\DuplicateCcliSongException;
|
||||
use App\Models\ApiRequestLog;
|
||||
use App\Models\Label;
|
||||
use App\Models\Setting;
|
||||
use App\Models\Song;
|
||||
use App\Models\SongArrangement;
|
||||
use App\Models\SongArrangementLabel;
|
||||
use App\Models\SongSlide;
|
||||
use App\Services\DTO\ParsedCcliSection;
|
||||
use App\Services\DTO\ParsedCcliSong;
|
||||
use App\Support\CcliLabels;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use RuntimeException;
|
||||
|
||||
final class CcliImportService
|
||||
{
|
||||
private const DEFAULT_LABEL_COLOR = '#3B82F6';
|
||||
|
||||
public function __construct(
|
||||
private readonly CcliPasteParser $parser,
|
||||
) {}
|
||||
|
||||
/** @return array{song: Song, status: 'created'|'restored', warnings: string[]} */
|
||||
public function import(string $rawText, ?string $sourceUrl = null): array
|
||||
{
|
||||
$startedAt = microtime(true);
|
||||
$parsed = $this->parser->parse($rawText);
|
||||
|
||||
if ($parsed->ccliId === null || trim($parsed->ccliId) === '') {
|
||||
throw new RuntimeException('Keine CCLI-Nummer gefunden — bitte vollständige SongSelect-Liedseite einfügen.');
|
||||
}
|
||||
|
||||
$song = Song::withTrashed()->where('ccli_id', $parsed->ccliId)->first();
|
||||
$status = 'created';
|
||||
|
||||
if ($song !== null && ! $song->trashed()) {
|
||||
throw new DuplicateCcliSongException($song->id);
|
||||
}
|
||||
|
||||
if ($song !== null) {
|
||||
$status = 'restored';
|
||||
}
|
||||
|
||||
return DB::transaction(function () use ($parsed, $sourceUrl, $song, $status, $startedAt): array {
|
||||
if ($song !== null && $song->trashed()) {
|
||||
$song->restore();
|
||||
}
|
||||
|
||||
$song = $this->upsertSong($parsed, $sourceUrl, $song);
|
||||
$warnings = [];
|
||||
|
||||
$translationLanguage = Setting::get('default_translation_language', 'DE');
|
||||
if ($translationLanguage === null || trim($translationLanguage) === '') {
|
||||
$warnings[] = 'Keine Standard-Übersetzungssprache gesetzt, DE wird verwendet.';
|
||||
}
|
||||
|
||||
$labelIds = [];
|
||||
$hasTranslation = false;
|
||||
|
||||
foreach ($parsed->sections as $section) {
|
||||
$label = $this->resolveLabel($section);
|
||||
$labelIds[] = $label->id;
|
||||
|
||||
$label->songSlides()->delete();
|
||||
|
||||
foreach ($section->lines as $order => $line) {
|
||||
$translatedLine = $section->linesTranslated[$order] ?? null;
|
||||
$hasTranslation = $hasTranslation || ($translatedLine !== null && trim($translatedLine) !== '');
|
||||
|
||||
SongSlide::create([
|
||||
'label_id' => $label->id,
|
||||
'order' => $order + 1,
|
||||
'text_content' => $line,
|
||||
'text_content_translated' => $translatedLine,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
$song->update([
|
||||
'has_translation' => $hasTranslation,
|
||||
'imported_from_ccli_at' => now(),
|
||||
'ccli_source_url' => $sourceUrl ?? $parsed->sourceUrl,
|
||||
]);
|
||||
|
||||
$arrangement = SongArrangement::updateOrCreate(
|
||||
['song_id' => $song->id, 'name' => 'normal'],
|
||||
['is_default' => true],
|
||||
);
|
||||
|
||||
SongArrangementLabel::where('song_arrangement_id', $arrangement->id)->delete();
|
||||
|
||||
foreach ($labelIds as $order => $labelId) {
|
||||
SongArrangementLabel::create([
|
||||
'song_arrangement_id' => $arrangement->id,
|
||||
'label_id' => $labelId,
|
||||
'order' => $order + 1,
|
||||
]);
|
||||
}
|
||||
|
||||
$song = $song->fresh(['arrangements.arrangementLabels.label.songSlides']);
|
||||
|
||||
ApiRequestLog::create([
|
||||
'method' => 'import',
|
||||
'endpoint' => 'paste',
|
||||
'status' => 'success',
|
||||
'request_context' => ['ccli_id' => $parsed->ccliId, 'mode' => $status],
|
||||
'response_summary' => "Song {$status}: {$song->title}",
|
||||
'response_body' => null,
|
||||
'duration_ms' => (int) round((microtime(true) - $startedAt) * 1000),
|
||||
]);
|
||||
|
||||
return ['song' => $song, 'status' => $status, 'warnings' => $warnings];
|
||||
});
|
||||
}
|
||||
|
||||
private function upsertSong(ParsedCcliSong $parsed, ?string $sourceUrl, ?Song $song): Song
|
||||
{
|
||||
$songData = [
|
||||
'title' => $parsed->title,
|
||||
'author' => $parsed->author,
|
||||
'copyright_text' => $parsed->copyrightText,
|
||||
'copyright_year' => $parsed->year,
|
||||
'publisher' => $parsed->copyrightText,
|
||||
'ccli_source_url' => $sourceUrl ?? $parsed->sourceUrl,
|
||||
];
|
||||
|
||||
if ($song !== null) {
|
||||
$song->update($songData);
|
||||
|
||||
return $song;
|
||||
}
|
||||
|
||||
return Song::create(array_merge($songData, ['ccli_id' => $parsed->ccliId]));
|
||||
}
|
||||
|
||||
private function resolveLabel(ParsedCcliSection $section): Label
|
||||
{
|
||||
$canonicalLabelName = CcliLabels::normalizeLabelName(
|
||||
$section->kind.($section->number ? ' '.$section->number : ''),
|
||||
);
|
||||
|
||||
return Label::firstOrCreate(
|
||||
['name' => $canonicalLabelName],
|
||||
['color' => self::DEFAULT_LABEL_COLOR, 'last_imported_at' => now()],
|
||||
);
|
||||
}
|
||||
}
|
||||
133
tests/Feature/CcliImportServiceTest.php
Normal file
133
tests/Feature/CcliImportServiceTest.php
Normal file
|
|
@ -0,0 +1,133 @@
|
|||
<?php
|
||||
|
||||
use App\Exceptions\DuplicateCcliSongException;
|
||||
use App\Models\ApiRequestLog;
|
||||
use App\Models\Song;
|
||||
use App\Models\SongArrangementLabel;
|
||||
use App\Models\SongSlide;
|
||||
use App\Services\CcliImportService;
|
||||
use Illuminate\Database\QueryException;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
function ccliFixture(string $name): string
|
||||
{
|
||||
return file_get_contents(base_path("tests/fixtures/ccli/{$name}"));
|
||||
}
|
||||
|
||||
test('imports english-only fixture and creates song with default arrangement', function () {
|
||||
$service = app(CcliImportService::class);
|
||||
|
||||
$result = $service->import(
|
||||
ccliFixture('english-only-multi-verse.txt'),
|
||||
'https://songselect.ccli.com/Songs/9999001',
|
||||
);
|
||||
|
||||
expect($result['status'])->toBe('created')
|
||||
->and($result['warnings'])->toBeArray()
|
||||
->and($result['song'])->toBeInstanceOf(Song::class);
|
||||
|
||||
$song = $result['song']->fresh();
|
||||
|
||||
expect($song->title)->toBe('Test Song 1')
|
||||
->and($song->author)->toBe('Test Artist 1')
|
||||
->and($song->ccli_id)->toBe('9999001')
|
||||
->and($song->copyright_year)->toBe('2024')
|
||||
->and($song->has_translation)->toBeFalse()
|
||||
->and($song->imported_from_ccli_at)->not->toBeNull()
|
||||
->and($song->ccli_source_url)->toBe('https://songselect.ccli.com/Songs/9999001');
|
||||
|
||||
$arrangement = $song->arrangements()->where('name', 'normal')->first();
|
||||
|
||||
expect($arrangement)->not->toBeNull()
|
||||
->and($arrangement->is_default)->toBeTrue()
|
||||
->and(SongArrangementLabel::where('song_arrangement_id', $arrangement->id)->count())->toBe(5)
|
||||
->and(SongSlide::count())->toBe(9);
|
||||
});
|
||||
|
||||
test('imports english and german fixture and stores translated slide text', function () {
|
||||
$service = app(CcliImportService::class);
|
||||
|
||||
$result = $service->import(ccliFixture('english-german-side-by-side.txt'));
|
||||
$song = $result['song']->fresh();
|
||||
|
||||
expect($result['status'])->toBe('created')
|
||||
->and($song->has_translation)->toBeTrue()
|
||||
->and(SongSlide::whereNotNull('text_content_translated')->count())->toBe(4)
|
||||
->and(SongSlide::where('text_content_translated', 'Deutsche Liedzeile 1 zum gleichen Gedanken')->exists())->toBeTrue();
|
||||
});
|
||||
|
||||
test('blocks active duplicate ccli_id with DuplicateCcliSongException', function () {
|
||||
$service = app(CcliImportService::class);
|
||||
$first = $service->import(ccliFixture('english-only-multi-verse.txt'));
|
||||
|
||||
expect(fn () => $service->import(ccliFixture('english-only-multi-verse.txt')))
|
||||
->toThrow(DuplicateCcliSongException::class);
|
||||
|
||||
try {
|
||||
$service->import(ccliFixture('english-only-multi-verse.txt'));
|
||||
} catch (DuplicateCcliSongException $exception) {
|
||||
expect($exception->existingSongId)->toBe($first['song']->id)
|
||||
->and($exception->getMessage())->toContain('existiert bereits');
|
||||
}
|
||||
|
||||
expect(Song::count())->toBe(1);
|
||||
});
|
||||
|
||||
test('restores soft-deleted song and does not duplicate normal arrangement', function () {
|
||||
$service = app(CcliImportService::class);
|
||||
$first = $service->import(ccliFixture('english-only-multi-verse.txt'));
|
||||
$songId = $first['song']->id;
|
||||
|
||||
Song::find($songId)->delete();
|
||||
|
||||
$result = $service->import(ccliFixture('english-only-multi-verse.txt'));
|
||||
$restoredSong = Song::withTrashed()->find($songId);
|
||||
|
||||
expect($result['status'])->toBe('restored')
|
||||
->and($result['song']->id)->toBe($songId)
|
||||
->and($restoredSong->trashed())->toBeFalse()
|
||||
->and($restoredSong->arrangements()->where('name', 'normal')->count())->toBe(1);
|
||||
});
|
||||
|
||||
test('throws RuntimeException when paste has no ccli id', function () {
|
||||
$content = "Test Song Title\nTest Artist\n\nVerse 1\nSome lyrics here\n\nChorus\nChorus lyrics\n\n© 2024 Publisher";
|
||||
$service = app(CcliImportService::class);
|
||||
|
||||
expect(fn () => $service->import($content))->toThrow(RuntimeException::class);
|
||||
expect(Song::count())->toBe(0);
|
||||
});
|
||||
|
||||
test('import creates ApiRequestLog with metadata only and no lyrics body', function () {
|
||||
$service = app(CcliImportService::class);
|
||||
|
||||
$service->import(ccliFixture('english-only-multi-verse.txt'));
|
||||
|
||||
$log = ApiRequestLog::latest()->first();
|
||||
|
||||
expect($log)->not->toBeNull()
|
||||
->and($log->method)->toBe('import')
|
||||
->and($log->endpoint)->toBe('paste')
|
||||
->and($log->status)->toBe('success')
|
||||
->and($log->request_context)->toMatchArray(['ccli_id' => '9999001', 'mode' => 'created'])
|
||||
->and($log->response_summary)->toBe('Song created: Test Song 1')
|
||||
->and($log->response_body)->toBeNull()
|
||||
->and($log->response_summary)->not->toContain('Morning light breaks');
|
||||
});
|
||||
|
||||
test('rolls back song and log when slide creation fails', function () {
|
||||
DB::statement("CREATE TRIGGER fail_ccli_slide_insert BEFORE INSERT ON song_slides BEGIN SELECT RAISE(ABORT, 'slide creation failed'); END");
|
||||
|
||||
try {
|
||||
expect(fn () => app(CcliImportService::class)->import(ccliFixture('english-only-multi-verse.txt')))
|
||||
->toThrow(QueryException::class);
|
||||
} finally {
|
||||
DB::statement('DROP TRIGGER IF EXISTS fail_ccli_slide_insert');
|
||||
}
|
||||
|
||||
expect(Song::count())->toBe(0)
|
||||
->and(SongSlide::count())->toBe(0)
|
||||
->and(ApiRequestLog::count())->toBe(0);
|
||||
});
|
||||
Loading…
Reference in a new issue