Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/parameters.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ parameters:
env(APP_DEV_EMAIL): 'dev@dev.com'
app.powered_by_phplist: '%%env(APP_POWERED_BY_PHPLIST)%%'
env(APP_POWERED_BY_PHPLIST): '0'
app.preference_page_show_private_lists: '%%env(PREFERENCEPAGE_SHOW_PRIVATE_LISTS)%%'
env(PREFERENCEPAGE_SHOW_PRIVATE_LISTS): '0'
app.rest_api_domain: '%%env(REST_API_DOMAIN)%%'
env(REST_API_DOMAIN): 'https://example.com/api/v2'

# Email configuration
app.mailer_from: '%%env(MAILER_FROM)%%'
Expand Down Expand Up @@ -115,6 +119,10 @@ parameters:
env(EXTERNALIMAGE_TIMEOUT): '30'
messaging.external_image_max_size: '%%env(EXTERNALIMAGE_MAXSIZE)%%'
env(EXTERNALIMAGE_MAXSIZE): '204800'
messaging.forward_alternative_content: '%%env(FORWARD_ALTERNATIVE_CONTENT)%%'
env(FORWARD_ALTERNATIVE_CONTENT): '0'
messaging.email_text_credits: '%%env(EMAILTEXTCREDITS)%%'
env(EMAILTEXTCREDITS): '0'

phplist.upload_images_dir: '%%env(PHPLIST_UPLOADIMAGES_DIR)%%'
env(PHPLIST_UPLOADIMAGES_DIR): 'images'
Expand Down
25 changes: 25 additions & 0 deletions config/services/builders.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,28 @@ services:
PhpList\Core\Domain\Messaging\Service\Builder\MessageOptionsBuilder:
autowire: true
autoconfigure: true

# Concrete mail constructors
PhpList\Core\Domain\Messaging\Service\Constructor\SystemMailConstructor: ~
PhpList\Core\Domain\Messaging\Service\Constructor\CampaignMailConstructor: ~

# Two EmailBuilder services with different constructors injected
Core.EmailBuilder.system:
class: PhpList\Core\Domain\Messaging\Service\Builder\EmailBuilder
arguments:
$mailConstructor: '@PhpList\Core\Domain\Messaging\Service\Constructor\SystemMailConstructor'
$googleSenderId: '%messaging.google_sender_id%'
$useAmazonSes: '%messaging.use_amazon_ses%'
$usePrecedenceHeader: '%messaging.use_precedence_header%'
$devVersion: '%app.dev_version%'
$devEmail: '%app.dev_email%'

Core.EmailBuilder.campaign:
class: PhpList\Core\Domain\Messaging\Service\Builder\EmailBuilder
arguments:
$mailConstructor: '@MailConstructor'
$googleSenderId: '%messaging.google_sender_id%'
$useAmazonSes: '%messaging.use_amazon_ses%'
$usePrecedenceHeader: '%messaging.use_precedence_header%'
$devVersion: '%app.dev_version%'
$devEmail: '%app.dev_email%'
4 changes: 4 additions & 0 deletions config/services/messenger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ services:

PhpList\Core\Domain\Messaging\MessageHandler\CampaignProcessorMessageHandler:
autowire: true
autoconfigure: true
arguments:
$emailBuilder: '@Core.EmailBuilder.system'
$messageEnvelope: '%app.config.message_from_address%'

PhpList\Core\Domain\Subscription\MessageHandler\DynamicTableMessageHandler:
autowire: true
Expand Down
5 changes: 5 additions & 0 deletions config/services/parameters.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
parameters:
# Flattened parameters for direct DI usage (Symfony does not support dot access into arrays)
app.config.message_from_address: 'news@example.com'
app.config.default_message_age: 15768000

# Keep original grouped array for legacy/config-provider usage
app.config:
message_from_address: 'news@example.com'
admin_address: 'admin@example.com'
Expand Down
5 changes: 5 additions & 0 deletions config/services/repositories.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ services:
arguments:
- PhpList\Core\Domain\Configuration\Model\EventLog

PhpList\Core\Domain\Configuration\Repository\UrlCacheRepository:
parent: PhpList\Core\Domain\Common\Repository\AbstractRepository
arguments:
- PhpList\Core\Domain\Configuration\Model\UrlCache


PhpList\Core\Domain\Identity\Repository\AdministratorRepository:
parent: PhpList\Core\Domain\Common\Repository\AbstractRepository
Expand Down
6 changes: 6 additions & 0 deletions config/services/resolvers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ services:
PhpList\Core\Bounce\Service\BounceActionResolver:
arguments:
- !tagged_iterator { tag: 'phplist.bounce_action_handler' }

_instanceof:
PhpList\Core\Domain\Configuration\Service\Placeholder\PlaceholderValueResolverInterface:
tags: ['phplist.placeholder_resolver']
PhpList\Core\Domain\Configuration\Service\Placeholder\PatternValueResolverInterface:
tags: [ 'phplist.pattern_resolver' ]
63 changes: 62 additions & 1 deletion config/services/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,39 @@ services:
autowire: true
autoconfigure: true

# Html to Text converter used by mail constructors
PhpList\Core\Domain\Common\Html2Text:
autowire: true
autoconfigure: true

# Rewrites relative asset URLs in fetched HTML to absolute ones
PhpList\Core\Domain\Common\HtmlUrlRewriter:
autowire: true
autoconfigure: true

# External image caching/downloading helper used by TemplateImageEmbedder
PhpList\Core\Domain\Common\ExternalImageService:
autowire: true
autoconfigure: true
arguments:
$tempDir: '%kernel.cache_dir%'
# Use literal defaults if parameters are not defined in this environment
$externalImageMaxAge: 0
$externalImageMaxSize: 204800
$externalImageTimeout: 30

# Embed images from templates and filesystem into HTML emails
PhpList\Core\Domain\Messaging\Service\TemplateImageEmbedder:
autowire: true
autoconfigure: true
arguments:
$documentRoot: '%kernel.project_dir%/public'
# Reuse upload_images_dir for editorImagesDir if a dedicated parameter is absent
$editorImagesDir: '%phplist.upload_images_dir%'
$embedExternalImages: '%messaging.embed_external_images%'
$embedUploadedImages: '%messaging.embed_uploaded_images%'
$uploadImagesDir: '%phplist.upload_images_dir%'

PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer:
autowire: true
autoconfigure: true
Expand Down Expand Up @@ -120,9 +153,12 @@ services:
autoconfigure: true
public: true

PhpList\Core\Domain\Configuration\Service\UserPersonalizer:
PhpList\Core\Domain\Configuration\Service\MessagePlaceholderProcessor:
autowire: true
autoconfigure: true
arguments:
$placeholderResolvers: !tagged_iterator phplist.placeholder_resolver
$patternResolvers: !tagged_iterator phplist.pattern_resolver

PhpList\Core\Domain\Configuration\Service\LegacyUrlBuilder:
autowire: true
Expand All @@ -139,3 +175,28 @@ services:
autoconfigure: true
arguments:
$maxMailSize: '%messaging.max_mail_size%'

# Loads and normalises message data for campaigns
PhpList\Core\Domain\Messaging\Service\MessageDataLoader:
autowire: true
autoconfigure: true
arguments:
$defaultMessageAge: '%app.config.default_message_age%'
Comment on lines +179 to +184
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Parameter name mismatch: app.config.default_message_age doesn't exist.

The parameter is defined as messaging.default_message_age in parameters.yml.dist (line 102-103), but here it's referenced as %app.config.default_message_age%. This will cause a container build error.

Fix the parameter reference
   PhpList\Core\Domain\Messaging\Service\MessageDataLoader:
     autowire: true
     autoconfigure: true
     arguments:
-      $defaultMessageAge: '%app.config.default_message_age%'
+      $defaultMessageAge: '%messaging.default_message_age%'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Loads and normalises message data for campaigns
PhpList\Core\Domain\Messaging\Service\MessageDataLoader:
autowire: true
autoconfigure: true
arguments:
$defaultMessageAge: '%app.config.default_message_age%'
# Loads and normalises message data for campaigns
PhpList\Core\Domain\Messaging\Service\MessageDataLoader:
autowire: true
autoconfigure: true
arguments:
$defaultMessageAge: '%messaging.default_message_age%'
🤖 Prompt for AI Agents
In @config/services/services.yml around lines 179 - 184, The service definition
for PhpList\Core\Domain\Messaging\Service\MessageDataLoader references a
non-existent parameter %app.config.default_message_age%; update the argument for
$defaultMessageAge to use the correct parameter name
%messaging.default_message_age% so the container can resolve the value.


# Common helpers required by precache/message building
PhpList\Core\Domain\Common\TextParser:
autowire: true
autoconfigure: true

PhpList\Core\Domain\Common\RemotePageFetcher:
autowire: true
autoconfigure: true

# Pre-caches base message content (HTML/Text/template) for campaigns
PhpList\Core\Domain\Messaging\Service\MessagePrecacheService:
autowire: true
autoconfigure: true
arguments:
$useManualTextPart: '%messaging.use_manual_text_part%'
$uploadImageDir: '%phplist.upload_images_dir%'
$publicSchema: '%phplist.public_schema%'
20 changes: 20 additions & 0 deletions resources/translations/messages.en.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,26 @@ Thank you.</target>
<source>phplist has started sending the campaign with subject %subject%</source>
<target>__phplist has started sending the campaign with subject %subject%</target>
</trans-unit>
<trans-unit id="PpLvt2Z" resname="Unsubscribe">
<source>Unsubscribe</source>
<target>__Unsubscribe</target>
</trans-unit>
<trans-unit id="njfOwly" resname="This link">
<source>This link</source>
<target>__This link</target>
</trans-unit>
<trans-unit id="7r3SSnf" resname="Confirm">
<source>Confirm</source>
<target>__Confirm</target>
</trans-unit>
<trans-unit id="YibbDEd" resname="Update preferences">
<source>Update preferences</source>
<target>__Update preferences</target>
</trans-unit>
<trans-unit id="BkWF7jw" resname="Sorry, you are not subscribed to any of our newsletters with this email address.">
<source>Sorry, you are not subscribed to any of our newsletters with this email address.</source>
<target>__Sorry, you are not subscribed to any of our newsletters with this email address.</target>
</trans-unit>
Comment on lines +753 to +772
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the translation file structure and count the __ pattern
wc -l resources/translations/messages.en.xlf

Repository: phpList/core

Length of output: 98


🏁 Script executed:

# Count how many target values have the __ prefix
rg '<target>__' resources/translations/messages.en.xlf | wc -l

Repository: phpList/core

Length of output: 58


🏁 Script executed:

# Check how many total target values exist
rg '<target>' resources/translations/messages.en.xlf | wc -l

Repository: phpList/core

Length of output: 59


🏁 Script executed:

# Look for the specific translation units mentioned in lines 753-772
sed -n '753,772p' resources/translations/messages.en.xlf

Repository: phpList/core

Length of output: 1015


🏁 Script executed:

# Search codebase for how translation strings are used/imported
fd -e ts -e js -e tsx -e jsx | head -20 | xargs rg 'messages\.en' 2>/dev/null | head -10

Repository: phpList/core

Length of output: 38


🏁 Script executed:

# Look for any configuration or documentation about the __ prefix
fd -e md -e txt -e json | xargs rg '__' 2>/dev/null | grep -i "translation\|prefix\|placeholder" | head -10

Repository: phpList/core

Length of output: 38


🏁 Script executed:

# Check the beginning of the translation file for any documentation
head -30 resources/translations/messages.en.xlf

Repository: phpList/core

Length of output: 1545


🏁 Script executed:

# Look for other language translation files
fd -e xlf resources/translations/

Repository: phpList/core

Length of output: 453


🏁 Script executed:

# Check one other language file to see if it also uses __ prefix
rg '<target>__' resources/translations/messages.*.xlf 2>/dev/null

Repository: phpList/core

Length of output: 6080


🏁 Script executed:

# Search for code that loads or processes XLIFF files
fd -e php -e ts -e js | xargs rg 'xlf|XLIFF|translation' 2>/dev/null | head -15

Repository: phpList/core

Length of output: 606


🏁 Script executed:

# Look at a sample of targets without __ prefix to understand the pattern
rg '<target>(?!__)' resources/translations/messages.en.xlf | head -5

Repository: phpList/core

Length of output: 281


🏁 Script executed:

# Check git history of the translation file to understand the pattern
git log --oneline -20 resources/translations/messages.en.xlf

Repository: phpList/core

Length of output: 79


🏁 Script executed:

# Look for any code that processes or strips the __ prefix
rg '__' src/ --type php --type ts --type js -B 2 -A 2 | head -30

Repository: phpList/core

Length of output: 2120


🏁 Script executed:

# Check if there's a pattern in how translations are loaded
rg 'xlf|Xliff|XLIFF' src/ --type php -B 2 -A 2

Repository: phpList/core

Length of output: 38


🏁 Script executed:

# Check if there's any configuration or setup file that mentions this pattern
fd -e yaml -e yml -e json -e md | xargs rg '__' 2>/dev/null | grep -i "translation\|xliff\|prefix" | head -10

Repository: phpList/core

Length of output: 38


🏁 Script executed:

# Look at more context around the file to understand intent
head -100 resources/translations/messages.en.xlf | tail -60

Repository: phpList/core

Length of output: 3674


🏁 Script executed:

# Check if there are other language files to compare
fd --full-path 'resources/translations/' . -name '*.xlf'

Repository: phpList/core

Length of output: 286


Remove the __ prefix from target values to match source strings.

The new translation units incorrectly prefix target values with __ (e.g., __Unsubscribe, __This link). For an English-to-English translation file, targets should match sources exactly. This inconsistency appears earlier in the file where entries are correctly formed—compare the new entries against lines like "Not authorized" which correctly have target matching source. If these strings are used in the application, the __ prefix will display to users as-is, breaking the UI text.

Diff showing required fixes:
-        <target>__Unsubscribe</target>
+        <target>Unsubscribe</target>

-        <target>__This link</target>
+        <target>This link</target>

-        <target>__Confirm</target>
+        <target>Confirm</target>

-        <target>__Update preferences</target>
+        <target>Update preferences</target>

-        <target>__Sorry, you are not subscribed to any of our newsletters with this email address.</target>
+        <target>Sorry, you are not subscribed to any of our newsletters with this email address.</target>
🤖 Prompt for AI Agents
In @resources/translations/messages.en.xlf around lines 753 - 772, The target
strings for the translation units PpLvt2Z, njfOwly, 7r3SSnf, YibbDEd, and
BkWF7jw incorrectly include a "__" prefix; update each <target> value to exactly
match its <source> (remove the "__" prefix) so "Unsubscribe", "This link",
"Confirm", "Update preferences", and "Sorry, you are not subscribed to any of
our newsletters with this email address." are used as the target values.

</body>
</file>
</xliff>
6 changes: 3 additions & 3 deletions src/Domain/Common/RemotePageFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function __construct(
) {
}

public function __invoke(string $url, array $userData): string
public function __invoke(string $url, array $userData): ?string
{
$url = $this->prepareUrl($url, $userData);

Expand Down Expand Up @@ -78,7 +78,7 @@ public function __invoke(string $url, array $userData): string
return $content;
}

private function fetchUrlDirect(string $url): string
private function fetchUrlDirect(string $url): ?string
{
try {
$response = $this->httpClient->request('GET', $url, [
Expand All @@ -88,7 +88,7 @@ private function fetchUrlDirect(string $url): string

return $response->getContent(false);
} catch (Throwable $e) {
return '';
return null;
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/Domain/Configuration/Model/ConfigOption.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ enum ConfigOption: string
case SubscribeMessage = 'subscribemessage';
case SubscribeEmailSubject = 'subscribesubject';
case UnsubscribeUrl = 'unsubscribeurl';
case BlacklistUrl = 'blacklisturl';
case ForwardUrl = 'forwardurl';
case ConfirmationUrl = 'confirmationurl';
case PreferencesUrl = 'preferencesurl';
case SubscribeUrl = 'subscribeurl';
// todo: check where is this defined
case Domain = 'domain';
Comment on lines +18 to 19
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider addressing or tracking this TODO.

This TODO comment was added in this PR, which suggests there's some uncertainty about the Domain config option. Might be worth either:

  1. Investigating where it's defined before merging, or
  2. Opening an issue to track this so it doesn't get lost in the codebase.

Want me to help search the codebase for where this domain configuration is defined or used?

🤖 Prompt for AI Agents
In @src/Domain/Configuration/Model/ConfigOption.php around lines 18 - 19, The
TODO on the ConfigOption enum for "case Domain = 'domain';" is unresolved;
either locate where the 'domain' config key is defined/used (search for the
literal 'domain' and inspect configuration loaders/services like
ConfigurationService/ConfigLoader) and replace the todo with a clear comment or
link to the defining code, or create a tracked issue and update the TODO to
reference that issue ID; ensure the symbolic enum member ConfigOption::Domain is
verified against the actual config schema and remove or replace the ambiguous
todo once confirmed.

case Website = 'website';
case MessageFromAddress = 'message_from_address';
Expand All @@ -33,4 +36,7 @@ enum ConfigOption: string
case PoweredByText = 'PoweredByText';
case UploadImageRoot = 'uploadimageroot';
case PageRoot = 'pageroot';
case OrganisationName = 'organisation_name';
case VCardUrl = 'vcardurl';
case HtmlEmailStyle = 'html_email_style';
}
44 changes: 44 additions & 0 deletions src/Domain/Configuration/Model/Dto/PlaceholderContext.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

declare(strict_types=1);

namespace PhpList\Core\Domain\Configuration\Model\Dto;

use PhpList\Core\Domain\Configuration\Model\OutputFormat;
use PhpList\Core\Domain\Subscription\Model\Subscriber;

final class PlaceholderContext
{
public function __construct(
public readonly Subscriber $user,
public readonly OutputFormat $format,
public readonly string $locale = 'en',
private readonly ?string $forwardedBy = null,
private readonly ?int $messageId = null,
) {}

public function isHtml(): bool
{
return $this->format === OutputFormat::Html;
}

public function isText(): bool
{
return $this->format === OutputFormat::Text;
}

public function forwardedBy(): ?string
{
return $this->forwardedBy;
}

public function messageId(): ?int
{
return $this->messageId;
}

public function getUser(): Subscriber
{
return $this->user;
}
}
11 changes: 11 additions & 0 deletions src/Domain/Configuration/Model/OutputFormat.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace PhpList\Core\Domain\Configuration\Model;

enum OutputFormat: string
{
case Html = 'html';
case Text = 'text';
}
12 changes: 11 additions & 1 deletion src/Domain/Configuration/Service/LegacyUrlBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,23 @@
class LegacyUrlBuilder
{
public function withUid(string $baseUrl, string $uid): string
{
return $this->withQueryParam($baseUrl, 'uid', $uid);
}

public function withEmail(string $baseUrl, string $email): string
{
return $this->withQueryParam($baseUrl, 'email', $email);
}

private function withQueryParam(string $baseUrl, string $paramName, string $paramValue): string
{
$parts = parse_url($baseUrl) ?: [];
$query = [];
if (!empty($parts['query'])) {
parse_str($parts['query'], $query);
}
$query['uid'] = $uid;
$query[$paramName] = $paramValue;

$parts['query'] = http_build_query($query);

Expand Down
Loading
Loading