Skip to content

fix(#299): stop TinyMCE double-prefixing URLs in email templates (and every editor) - #300

Merged
fabiodalez-dev merged 2 commits into
mainfrom
fix/email-template-url-prefix-299
Jul 28, 2026
Merged

fix(#299): stop TinyMCE double-prefixing URLs in email templates (and every editor)#300
fabiodalez-dev merged 2 commits into
mainfrom
fix/email-template-url-prefix-299

Conversation

@fabiodalez-dev

@fabiodalez-dev fabiodalez-dev commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Problem

#299: email templates come out with links double-prefixed by the admin base after the template is edited once in the WYSIWYG editor. An href of {{login_url}} is saved as https://host/admin/{{login_url}}, so the sent email link becomes https://host/admin/https://host/accedi.

Root cause

TinyMCE's convert_urls (on by default) resolves every href against the editing page's base URL — which is the admin page — so it prefixes links, and even placeholder tokens like {{login_url}}, with /admin. On re-edit it compounds.

relative_urls: false alone does not prevent this: it only chooses absolute vs relative output for a conversion that still happens. The correct switch is convert_urls: false, which tells TinyMCE to leave URLs exactly as written.

Fix

  • convert_urls: false on all six TinyMCE inits: settings/index (email templates — the reported case), book_form (#descrizione), events/form (#event_content), cms/edit-home (#text_content_body), admin/cms-edit (#tinymce-editor), and the legacy admin/settings view (dead code — not routed — fixed for consistency).
  • Heal already-corrupted templates. EmailService::replaceVariables now strips an absolute …/admin/ prefix sitting directly in front of a {{placeholder}} before substitution, so existing installs (like the reporter's) don't need every template re-saved after updating. A legitimate template never puts a host+/admin/ in front of a token, so this is safe.

Tests

Test Covers
tests/email-template-url-prefix-299.unit.php (9) The healing: corrupted href → correct single URL; http/https; subdir base; multiple links; no-ops for real /admin/ URLs and non-URL placeholders.
tests/tinymce-url-prefix-299.spec.js (4) Real browser feeds an href with a placeholder through each editor and asserts convert_urls is false AND the placeholder round-trips verbatim — no /admin/ prefix, no %7B%7B encoding.

All green. Full-tree PHPStan level 5 clean.

Closes #299.

Summary by CodeRabbit

  • Correzioni

    • Risolto il problema dei doppi prefissi /admin/ nei link con segnaposto dei template email.
    • Gli URL nei contenuti modificati con l’editor vengono ora preservati senza riscritture o prefissi indesiderati.
    • I template già interessati dal problema vengono automaticamente riparati durante la sostituzione dei segnaposto.
  • Test

    • Aggiunti controlli automatici per verificare la corretta gestione degli URL nei template email e negli editor amministrativi.

… every editor)

The email-template WYSIWYG editor saved links double-prefixed with the admin
base, e.g. an href of {{login_url}} came back as
https://host/admin/{{login_url}}, so the sent email link resolved to
https://host/admin/https://host/accedi.

Root cause: TinyMCE's convert_urls (on by default) resolves every href against
the editing page's base URL — the admin page — prefixing links and even
placeholder tokens with /admin. relative_urls:false alone does NOT prevent this;
it only picks absolute vs relative for the (still-performed) conversion. The fix
is convert_urls:false, which tells TinyMCE never to rewrite URLs.

- Set convert_urls:false on ALL six TinyMCE inits: settings/index (email
  templates — the reported case), book_form (#descrizione), events/form
  (#event_content), cms/edit-home (#text_content_body), admin/cms-edit
  (#tinymce-editor), and the legacy admin/settings view (dead code — not routed
  anywhere — fixed for consistency).
- EmailService::replaceVariables now heals templates ALREADY saved corrupted on
  existing installs: it strips an absolute "…/admin/" prefix sitting directly in
  front of a {{placeholder}} before substitution, so a user doesn't have to
  re-save every template after updating. A legitimate template never puts a
  host+/admin/ in front of a token, so this is safe.

Tests:
- tests/email-template-url-prefix-299.unit.php (9) — the healing: corrupted
  href → correct single URL, http/https, subdir base, multiple links, and
  no-ops for real /admin/ URLs and non-URL placeholders.
- tests/tinymce-url-prefix-299.spec.js (4) — real browser feeds an href with a
  placeholder through each editor and asserts convert_urls is false AND the
  placeholder round-trips verbatim with no /admin/ prefix and no %7B%7B encoding.
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@fabiodalez-dev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5ed1a77c-f751-41a2-9d9a-010e6b1dffbf

📥 Commits

Reviewing files that changed from the base of the PR and between 103ff18 and a17f57e.

📒 Files selected for processing (4)
  • app/Controllers/SettingsController.php
  • app/Models/SettingsRepository.php
  • app/Support/EmailService.php
  • tests/tinymce-url-prefix-299.spec.js
📝 Walkthrough

Walkthrough

La modifica impedisce a TinyMCE di riscrivere gli URL nei contenuti amministrativi e aggiunge una riparazione in EmailService::replaceVariables() per i template già corrotti. Sono inclusi test PHP e Playwright per verificare placeholder, prefissi /admin/ e URL assoluti.

Changes

Correzione dei prefissi URL

Layer / File(s) Summary
Preservazione degli URL negli editor
app/Views/admin/cms-edit.php, app/Views/admin/settings.php, app/Views/cms/edit-home.php, app/Views/events/form.php, app/Views/libri/partials/book_form.php, app/Views/settings/index.php
Le configurazioni TinyMCE impostano convert_urls: false per evitare la riscrittura degli URL rispetto alla base admin.
Riparazione dei template esistenti
app/Support/EmailService.php, tests/email-template-url-prefix-299.unit.php
replaceVariables() rimuove il prefisso /admin/ duplicato davanti ai placeholder URL; il test verifica URL HTTP/HTTPS, sottocartelle, casi invariati e più link nello stesso contenuto.
Test end-to-end degli editor
tests/tinymce-url-prefix-299.spec.js
Il test Playwright verifica tutti gli editor interessati e conferma che {{login_url}} resti invariato, senza prefissi aggiunti né codifica delle parentesi.

Estimated code review effort: 3 (Moderate) | ~20 minuti

Possibly related PRs

  • fabiodalez-dev/Pinakes#207: modifica anch’essa EmailService::replaceVariables() nella gestione di placeholder e link dei template email.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Il titolo descrive correttamente il fix del doppio prefisso TinyMCE nelle email e negli editor coinvolti.
Linked Issues check ✅ Passed La modifica rimuove il prefisso /admin/ davanti ai placeholder e impedisce a TinyMCE di riscrivere gli URL, centrando #299.
Out of Scope Changes check ✅ Passed Le modifiche agli altri editor e i test aggiunti restano coerenti con l'obiettivo del fix e non appaiono fuori ambito.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/email-template-url-prefix-299

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/tinymce-url-prefix-299.spec.js`:
- Line 47: Rimuovi l’attesa page.waitForLoadState('networkidle') nel test e
affidati al successivo waitForFunction per attendere la condizione applicativa
necessaria. Mantieni invariato il resto del flusso del test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eac3d7a7-a1df-4def-9bdd-b664e11904c0

📥 Commits

Reviewing files that changed from the base of the PR and between c5905bf and 103ff18.

📒 Files selected for processing (9)
  • app/Support/EmailService.php
  • app/Views/admin/cms-edit.php
  • app/Views/admin/settings.php
  • app/Views/cms/edit-home.php
  • app/Views/events/form.php
  • app/Views/libri/partials/book_form.php
  • app/Views/settings/index.php
  • tests/email-template-url-prefix-299.unit.php
  • tests/tinymce-url-prefix-299.spec.js

Comment thread tests/tinymce-url-prefix-299.spec.js Outdated
… at render

Follow-up so existing installs get their STORED template values fixed, not only
the rendered email. TinyMCE no longer rewrites URLs (convert_urls:false), and
EmailService heals at render time, but a template saved before the fix keeps the
double-prefixed link in the DB — visible in the editor and wrong if read directly.

- Extract the render-time repair into EmailService::healPlaceholderUrls() (static,
  reused by the DB heal — one place for the pattern).
- SettingsRepository::healCorruptedTemplateUrls() repairs stored rows: a cheap
  LIKE '%/admin/{{%' narrows to affected templates, the fix is done in PHP
  (portable across MySQL 5.7+/MariaDB — no REGEXP_REPLACE, and the host varies so
  a literal REPLACE won't do). Idempotent, all locales.
- SettingsController runs it right after ensureEmailTemplates, so opening the
  settings page repairs the stored values.

Also address CodeRabbit: drop waitForLoadState('networkidle') (flaky) from the
E2E — the following waitForFunction is the real readiness signal.

Tests: tinymce-url-prefix-299.spec.js adds a DB-repair case — a template stored
with an /admin/ prefix is healed to the plain placeholder after the settings page
is opened. unit suite unchanged (9) and still green after the extraction.
@fabiodalez-dev

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@fabiodalez-dev
fabiodalez-dev merged commit 0f4ad5f into main Jul 28, 2026
6 checks passed
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.

Email templates are double prefixed (after being edited first time)

1 participant