-
Notifications
You must be signed in to change notification settings - Fork 31
Feat/user personalizer #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ref/campaign-processing
Are you sure you want to change the base?
Changes from all commits
a2d50a1
216978e
968044a
78146e5
975b418
9b45ab3
8ed86ed
5bed8e2
5fdae5b
9d07526
de8f4a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's examine the translation file structure and count the __ pattern
wc -l resources/translations/messages.en.xlfRepository: phpList/core Length of output: 98 🏁 Script executed: # Count how many target values have the __ prefix
rg '<target>__' resources/translations/messages.en.xlf | wc -lRepository: phpList/core Length of output: 58 🏁 Script executed: # Check how many total target values exist
rg '<target>' resources/translations/messages.en.xlf | wc -lRepository: 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.xlfRepository: 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 -10Repository: 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 -10Repository: phpList/core Length of output: 38 🏁 Script executed: # Check the beginning of the translation file for any documentation
head -30 resources/translations/messages.en.xlfRepository: 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/nullRepository: 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 -15Repository: 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 -5Repository: 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.xlfRepository: 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 -30Repository: 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 2Repository: 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 -10Repository: 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 -60Repository: 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 The new translation units incorrectly prefix target values with Diff showing required fixes:🤖 Prompt for AI Agents |
||
| </body> | ||
| </file> | ||
| </xliff> | ||
| 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; | ||
| } | ||
| } |
| 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'; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| declare(strict_types=1); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| namespace PhpList\Core\Domain\Configuration\Service\Placeholder; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| use PhpList\Core\Domain\Configuration\Model\ConfigOption; | ||||||||||||||||||||||||||||||||||||||||||||||
| use PhpList\Core\Domain\Configuration\Service\LegacyUrlBuilder; | ||||||||||||||||||||||||||||||||||||||||||||||
| use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider; | ||||||||||||||||||||||||||||||||||||||||||||||
| use PhpList\Core\Domain\Configuration\Model\Dto\PlaceholderContext; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| final class BlacklistUrlValueResolver implements PlaceholderValueResolverInterface | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| public function __construct( | ||||||||||||||||||||||||||||||||||||||||||||||
| private readonly ConfigProvider $config, | ||||||||||||||||||||||||||||||||||||||||||||||
| private readonly LegacyUrlBuilder $urlBuilder, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) {} | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| public function name(): string | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| return 'BLACKLISTURL'; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| public function __invoke(PlaceholderContext $ctx): string | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| $base = $this->config->getValue(ConfigOption::UnsubscribeUrl) ?? ''; | ||||||||||||||||||||||||||||||||||||||||||||||
| $url = $this->urlBuilder->withEmail($base, $ctx->getUser()->getEmail()); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if ($ctx->isHtml()) { | ||||||||||||||||||||||||||||||||||||||||||||||
| return htmlspecialchars($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| return $url; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likely bug: should use Line 26 uses For comparison, 🐛 Proposed fix public function __invoke(PlaceholderContext $ctx): string
{
- $base = $this->config->getValue(ConfigOption::UnsubscribeUrl) ?? '';
+ $base = $this->config->getValue(ConfigOption::BlacklistUrl) ?? '';
$url = $this->urlBuilder->withEmail($base, $ctx->getUser()->getEmail());
if ($ctx->isHtml()) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace PhpList\Core\Domain\Configuration\Service\Placeholder; | ||
|
|
||
| use PhpList\Core\Domain\Configuration\Model\ConfigOption; | ||
| use PhpList\Core\Domain\Configuration\Service\LegacyUrlBuilder; | ||
| use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider; | ||
| use PhpList\Core\Domain\Configuration\Model\Dto\PlaceholderContext; | ||
| use Symfony\Contracts\Translation\TranslatorInterface; | ||
|
|
||
| final class BlacklistValueResolver implements PlaceholderValueResolverInterface | ||
| { | ||
| public function __construct( | ||
| private readonly ConfigProvider $config, | ||
| private readonly LegacyUrlBuilder $urlBuilder, | ||
| private readonly TranslatorInterface $translator, | ||
| ) {} | ||
|
|
||
| public function name(): string | ||
| { | ||
| return 'BLACKLIST'; | ||
| } | ||
|
|
||
| public function __invoke(PlaceholderContext $ctx): string | ||
| { | ||
| $base = $this->config->getValue(ConfigOption::BlacklistUrl) ?? ''; | ||
| $url = $this->urlBuilder->withEmail($base, $ctx->getUser()->getEmail()); | ||
|
|
||
| if ($ctx->isHtml()) { | ||
| $label = $this->translator->trans('Unsubscribe'); | ||
| $safeLabel = htmlspecialchars($label, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); | ||
| $safeUrl = htmlspecialchars($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); | ||
|
|
||
| return '<a href="' . $safeUrl . '">' . $safeLabel . '</a>'; | ||
| } | ||
|
|
||
| return $url; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: phpList/core
Length of output: 178
🏁 Script executed:
Repository: phpList/core
Length of output: 38
🏁 Script executed:
Repository: phpList/core
Length of output: 117
🏁 Script executed:
Repository: phpList/core
Length of output: 6367
🏁 Script executed:
Repository: phpList/core
Length of output: 595
String-to-boolean conversion will cause incorrect behavior.
The parameter
$preferencePageShowPrivateListsis type-hinted asbool(line 28 of UserPersonalizer.php), but the services configuration passes a string value from parameters.yml.dist. In PHP,(bool)'0'evaluates totrue(non-empty string), notfalse. This means the feature will behave opposite to the intended default—always showing private lists when it should hide them.Use an integer instead of a string in parameters.yml.dist:
Or apply explicit casting in services.yml using
!php/constor a cast operator.🤖 Prompt for AI Agents