Skip to content

Conversation

escopecz
Copy link
Member

@escopecz escopecz commented Apr 16, 2025

Q A
Bug fix? (use the a.b branch)
New feature/enhancement? (use the a.x branch) ✔️
Deprecations? maybe in Symfony
BC breaks? (use the c.x branch) ✔️
Automated tests included?
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

The composer changes were generated with command composer update symfony/* klapaudius/oauth-server-bundle

It generated this output:

Lock file operations: 1 install, 46 updates, 0 removals
  - Upgrading klapaudius/oauth-server-bundle (4.0.5 => 5.1.1)
  - Upgrading mautic/core-lib (7.0.0-dev 4b70c52 => 7.0.0-dev 8ca68dd)
  - Upgrading symfony/asset (v6.4.13 => v7.2.0)
  - Upgrading symfony/browser-kit (v6.4.13 => v6.4.19)
  - Upgrading symfony/cache (v6.4.16 => v7.2.5)
  - Upgrading symfony/clock (v6.4.13 => v7.2.0)
  - Upgrading symfony/config (v6.4.14 => v7.2.3)
  - Upgrading symfony/console (v6.4.17 => v7.2.5)
  - Upgrading symfony/dependency-injection (v6.4.19 => v7.2.5)
  - Upgrading symfony/doctrine-bridge (v6.4.16 => v7.2.5)
  - Upgrading symfony/doctrine-messenger (v5.4.45 => v7.2.5)
  - Upgrading symfony/dom-crawler (v6.4.16 => v6.4.19)
  - Upgrading symfony/dotenv (v6.4.16 => v7.2.0)
  - Upgrading symfony/error-handler (v6.4.14 => v7.2.5)
  - Upgrading symfony/event-dispatcher (v6.4.13 => v7.2.0)
  - Upgrading symfony/expression-language (v6.4.13 => v7.2.0)
  - Upgrading symfony/filesystem (v6.4.13 => v7.2.0)
  - Upgrading symfony/finder (v6.4.17 => v7.2.2)
  - Upgrading symfony/form (v6.4.13 => v7.2.5)
  - Upgrading symfony/framework-bundle (v6.4.13 => v7.2.5)
  - Upgrading symfony/http-client (v6.4.16 => v7.2.4)
  - Upgrading symfony/http-foundation (v6.4.16 => v7.2.5)
  - Upgrading symfony/http-kernel (v6.4.16 => v7.2.5)
  - Upgrading symfony/intl (v6.4.15 => v7.2.0)
  - Upgrading symfony/lock (v6.4.13 => v7.2.5)
  - Upgrading symfony/mailer (v6.4.13 => v7.2.3)
  - Upgrading symfony/maker-bundle (v1.61.0 => v1.62.1)
  - Upgrading symfony/messenger (v6.4.16 => v7.2.5)
  - Upgrading symfony/mime (v6.4.13 => v7.2.4)
  - Upgrading symfony/options-resolver (v6.4.16 => v7.2.0)
  - Upgrading symfony/process (v6.4.19 => v7.2.5)
  - Upgrading symfony/property-access (v6.4.13 => v7.2.3)
  - Upgrading symfony/property-info (v6.4.16 => v7.2.5)
  - Upgrading symfony/routing (v6.4.16 => v7.2.3)
  - Upgrading symfony/security-bundle (v6.4.13 => v7.2.3)
  - Upgrading symfony/security-core (v6.4.18 => v7.2.3)
  - Upgrading symfony/security-csrf (v6.4.13 => v7.2.3)
  - Upgrading symfony/security-http (v6.4.15 => v7.2.4)
  - Upgrading symfony/serializer (v6.4.18 => v7.2.5)
  - Upgrading symfony/stopwatch (v6.4.13 => v7.2.4)
  - Upgrading symfony/translation (v6.4.13 => v7.2.4)
  - Upgrading symfony/twig-bridge (v6.4.16 => v7.2.5)
  - Upgrading symfony/twig-bundle (v6.4.13 => v7.2.0)
  - Locking symfony/type-info (v7.2.5)
  - Upgrading symfony/validator (v6.4.16 => v7.2.5)
  - Upgrading symfony/var-exporter (v6.4.13 => v7.2.5)
  - Downgrading symfony/web-profiler-bundle (v6.4.16 => v6.4.0-BETA1)

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Nothing should change, everything should work. This change upgrades the underlying framework to a new major version so it can affect everything.

@escopecz escopecz added WIP PR's that are not ready for review and are currently in progress bc-break A BC break PR for major release milestones only dependencies Pull requests that update a dependency file labels Apr 16, 2025
@matbcvo matbcvo added this to the 7.0.0-alpha milestone Apr 16, 2025
@matbcvo matbcvo added the essential This must be done to close the milestone label Apr 29, 2025
@escopecz escopecz self-assigned this Apr 29, 2025
@escopecz
Copy link
Member Author

I'm trying to resolve the migrations issue in #14954 to get the PHPSTAN checks green

@matbcvo
Copy link
Contributor

matbcvo commented May 29, 2025

I have resolved 72 PHPStan errors, and there is 1 remaining (or 2, including the migration DI issue, which will be fixed in a separate PR). I'm unsure how to resolve this one. Should PHPStan ignore it?

https://github.com/mautic/mautic/blob/7.x/app/bundles/CoreBundle/Translation/TranslatorLoader.php

The Translator class has been marked as final since Symfony 7.1, as indicated in the docblock.

https://github.com/symfony/framework-bundle/blob/7.2/Translation/Translator.php

 ------ ---------------------------------------------------------------------------------------------------------------------------------- 
  Line   app/bundles/CoreBundle/Translation/TranslatorLoader.php                                                                           
 ------ ---------------------------------------------------------------------------------------------------------------------------------- 
  7      Class Mautic\CoreBundle\Translation\TranslatorLoader extends @final class Symfony\Bundle\FrameworkBundle\Translation\Translator.  
         🪪  class.extendsFinalByPhpDoc                                                                                                    
 ------ ---------------------------------------------------------------------------------------------------------------------------------- 
                                                                    
 [ERROR] Found 1 error

@escopecz
Copy link
Member Author

@matbcvo I think we should refactor to decorator pattern. It would fail with a PHP error if we wouldn't right? This is what AI dumped:

<?php

declare(strict_types=1);

namespace Mautic\CoreBundle\Translation;

use Symfony\Bundle\FrameworkBundle\Translation\Translator as FrameworkTranslator;
use Symfony\Component\Translation\MessageCatalogueInterface;

class TranslatorLoader implements TranslatorInterface, LocaleAwareInterface
{
    private FrameworkTranslator $translator;

    public function __construct(FrameworkTranslator $translator)
    {
        $this->translator = $translator;
    }

    public function trans(string $id, array $parameters = [], string $domain = null, string $locale = null): string
    {
        return $this->translator->trans($id, $parameters, $domain, $locale);
    }

    public function getCatalogue($locale = null): MessageCatalogueInterface
    {
        // Custom logic before calling parent
        if ('en_US' !== $locale && !$this->translator->getCatalogue('en_US')->has('loaded')) {
            $this->translator->addResource('mautic', null, 'en_US', 'messages');
        }
        
        $this->translator->addResource('mautic', null, $locale, 'messages');
        
        return $this->translator->getCatalogue($locale);
    }

    // Delegate other required interface methods...
}
<?php

namespace Mautic\CoreBundle\DependencyInjection\Compiler;

use Mautic\CoreBundle\Translation\TranslatorDecorator;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;

class TranslationLoaderPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if (!$container->has('translator.default')) {
            return;
        }

        // Create decorator service
        $decoratorDefinition = new Definition(TranslatorDecorator::class);
        $decoratorDefinition->setDecoratedService('translator.default');
        $decoratorDefinition->setArgument(0, new Reference('translator.default.inner'));
        $decoratorDefinition->setPublic(true);
        
        $container->setDefinition('mautic.translator.decorator', $decoratorDefinition);
    }
}

@escopecz escopecz assigned escopecz and unassigned escopecz Jul 9, 2025
@escopecz escopecz moved this to Todo in Quality Sprint 2025 Jul 9, 2025
@escopecz escopecz moved this from Todo to In Progress in Quality Sprint 2025 Jul 9, 2025
@escopecz escopecz marked this pull request as ready for review July 10, 2025 10:12
Copy link

codecov bot commented Jul 10, 2025

Codecov Report

Attention: Patch coverage is 74.48980% with 25 lines in your changes missing coverage. Please review.

Project coverage is 66.33%. Comparing base (fd38b8f) to head (62d5ef2).
Report is 53 commits behind head on 7.x.

Files with missing lines Patch % Lines
...es/CoreBundle/Doctrine/AbstractMauticMigration.php 50.00% 2 Missing ⚠️
...ntity/Transformer/NotificationArrayTransformer.php 0.00% 2 Missing ⚠️
...rm/DataTransformer/DatetimeToStringTransformer.php 0.00% 2 Missing ⚠️
...le/Form/DataTransformer/EmojiToHtmlTransformer.php 0.00% 2 Missing ⚠️
...le/Form/DataTransformer/NullToEmptyTransformer.php 0.00% 2 Missing ⚠️
app/bundles/CoreBundle/Translation/Translator.php 0.00% 2 Missing ⚠️
.../Form/DataTransformer/EventsToArrayTransformer.php 0.00% 2 Missing ⚠️
...bundles/CoreBundle/Command/ApplyUpdatesCommand.php 0.00% 1 Missing ⚠️
...s/CoreBundle/Command/CleanupMediaAssetsCommand.php 0.00% 1 Missing ⚠️
...undles/CoreBundle/Command/ConvertConfigCommand.php 0.00% 1 Missing ⚠️
... and 8 more
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #14887      +/-   ##
============================================
- Coverage     66.48%   66.33%   -0.16%     
+ Complexity    35185    35169      -16     
============================================
  Files          2317     2317              
  Lines        141873   141765     -108     
============================================
- Hits          94329    94040     -289     
- Misses        47544    47725     +181     
Files with missing lines Coverage Δ
.../bundles/CacheBundle/Command/ClearCacheCommand.php 0.00% <ø> (-66.67%) ⬇️
...nBundle/Command/CampaignDeleteEventLogsCommand.php 100.00% <100.00%> (ø)
...les/CampaignBundle/Command/ExecuteEventCommand.php 100.00% <ø> (ø)
...undles/CampaignBundle/Command/SummarizeCommand.php 97.22% <100.00%> (ø)
.../CampaignBundle/Command/TriggerCampaignCommand.php 87.03% <ø> (-0.06%) ⬇️
...paignBundle/Command/UpdateLeadCampaignsCommand.php 75.91% <ø> (-0.35%) ⬇️
...es/CampaignBundle/Command/ValidateEventCommand.php 86.04% <ø> (-0.32%) ⬇️
...e/Command/ProcessMarketingMessagesQueueCommand.php 90.00% <ø> (-1.90%) ⬇️
...nnelBundle/Command/SendChannelBroadcastCommand.php 85.43% <100.00%> (ø)
...ndles/ConfigBundle/Form/Type/EscapeTransformer.php 96.00% <100.00%> (ø)
... and 81 more

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@escopecz escopecz requested a review from fedys July 10, 2025 12:13
Copy link
Member

@fedys fedys left a comment

Choose a reason for hiding this comment

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

Thank you @escopecz!

Copy link

@fedys fedys requested a review from matbcvo July 10, 2025 15:18
@escopecz escopecz moved this from In Progress to Needs review in Quality Sprint 2025 Jul 10, 2025
@matbcvo matbcvo removed the WIP PR's that are not ready for review and are currently in progress label Jul 10, 2025
Copy link
Contributor

@matbcvo matbcvo left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. I have tested the PR and found no issues. 👍 Thanks @escopecz and @fedys!

@escopecz escopecz added the enhancement Any improvement to an existing feature or functionality label Jul 10, 2025
@escopecz escopecz merged commit 32f3555 into mautic:7.x Jul 10, 2025
22 of 23 checks passed
@github-project-automation github-project-automation bot moved this from Needs review to Done in Quality Sprint 2025 Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bc-break A BC break PR for major release milestones only dependencies Pull requests that update a dependency file enhancement Any improvement to an existing feature or functionality essential This must be done to close the milestone
Projects
Development

Successfully merging this pull request may close these issues.

3 participants