Problem/Motivation

Since the module's 4.0 release many new features were added to PHP and Drupal core which could make the module's code more compact, maintainable without lots of boilerplate code such as hook classes, autowiring services, constructor property promotion, changing plugin annotations to PHP attributes etc.

Consider doing that for a new 4.1.x minor version (or a new major version if some BC break is needed for plugins) with raising the minimum core version to 11.3 or later.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork google_analytics_counter-3628928

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kaszarobert created an issue. See original summary.

avattikonda’s picture

avattikonda
Primary language English
Location Boston
commented
Assigned: Unassigned » avattikonda

avattikonda opened merge request !32

kaszarobert’s picture

kaszarobert
Primary language Hungarian
commented
Status: Active » Needs work

Hm, right now /admin/config/system/google-analytics-counter/dashboard shows this due to declare(strict_types=1) causing TypeError on number_format():

TypeError: number_format(): Argument #1 ($num) must be of type int|float, string given in number_format() (line 125 of modules/contrib/google_analytics_counter/src/Controller/GoogleAnalyticsCounterController.php).
Drupal\google_analytics_counter\Controller\GoogleAnalyticsCounterController->dashboard()
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::wrapControllerExecutionInRenderContext():121}() (Line: 638)
Drupal\Core\Render\Renderer::{closure:Drupal\Core\Render\Renderer::executeInRenderContext():638}()
Fiber->start() (Line: 639)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->{closure:Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber::onController():96}() (Line: 185)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 30)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 118)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 92)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 71)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 753)
Drupal\Core\DrupalKernel->handle(Object) (Line: 34)
Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() (Line: 32)
require('/app/vendor/autoload_runtime.php') (Line: 22)
require_once('/app/web/autoload_runtime.php') (Line: 13)

Also, according to a similar issue on another project we cannot use Read-only properties as they could cause serialization issues on forms later until the minimum supported PHP is 8.4. And the mentioned minimum Drupal 11 should mean PHP 8.3 is the minimum version to be supported.

skip_procedural_hook_scan: true options turns off every other hooks in *.views.inc, *.install not just in *.module AFAIK. That should be fixed.

Since the many recent regression issues that were not caught during our manual code reviews please wait with working on this until proper test coverage is committed in #3628929: Better test coverage. Update: done, rebase the branch and please make sure no code is lost during rebase. The tests are checking everything the module does now so they should show if there are regressions immediately.