Skip to content

Fix PR review issues: head.html test content, misleading test name, and missing try/finally - #22

Draft
printminion-co with Copilot wants to merge 3 commits into
kh/dev/brand-aware-templatesfrom
copilot/sub-pr-21
Draft

printminion-co with Copilot wants to merge 3 commits into
kh/dev/brand-aware-templatesfrom
copilot/sub-pr-21

Conversation

Copilot AI commented Mar 10, 2026

Copy link
Copy Markdown

Three issues flagged in code review on the brand-aware templates PR.

Changes

  • lib/templates/email/head.html – Replaced committed test fixture string (<div>Test Head Template</div>) with a backward-compatibility comment; brand-specific templates under ionos/ are the intended path.

  • tests/unit/BrandResolverTest.php – Renamed testGetBrandReturnsDefaultWhenNotConfiguredtestGetBrandReturnsDefaultBrand (the test explicitly passes 'ionos', so it validates the default brand value, not an unconfigured state); assertion updated to use BrandResolver::DEFAULT_BRAND instead of the hardcoded string.

  • tests/lib/EMailTemplateTest.php – Wrapped the file_put_contents write and all assertions in testIncludeTemplateFileIsCovered with try/finally to guarantee file restoration on assertion failure or exception:

file_put_contents($templateFile, $expectedContent);
try {
    // ... reset property, invoke method, assert ...
} finally {
    if ($originalContent !== null) {
        file_put_contents($templateFile, $originalContent);
    } else {
        unlink($templateFile);
    }
}

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

bromiesTM and others added 2 commits March 6, 2026 14:01
…olution

Signed-off-by: Kai Henseler <kai.henseler@strato.de>
Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com>
Copilot AI changed the title [WIP] Add brand-aware email templates with asset resolution Fix PR review issues: head.html test content, misleading test name, and missing try/finally Mar 10, 2026
@bromiesTM
bromiesTM force-pushed the kh/dev/brand-aware-templates branch from 58d1dd3 to 911878c Compare March 16, 2026 08:22
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