202411 consentratio lp 135 #495

anthonylindsay · 2024-11-11T10:45:17Z

Adds a custom module to track cookie consent choices to the database without GA.
Loads Javascript to listen for EU Cookie choice selection, logs choice and timestamp to the DB.
Tracking can be turned on or off via settings screen.
https://eccservicetransformation.atlassian.net/browse/LP-135

Polynya · 2024-11-20T12:07:06Z

web/modules/custom/ecc_cct/config/schema/ecc_cct.schema.yml

+  label: 'ECC Cookie Consent Tracker settings'
+  mapping:
+    enable:
+      type: string


Why not boolean?

The exact why is lost in the mists of history, but I'm guessing it's an artifact of drush generate:module. It really doesn't matter one way or the other. It's stored as a string, it's read as a string, it works. I don't see it as having much impact either way? I mean, we can change it, but it'll be change for change's sake.

Generate may be quick but it's no guarantee of good code.

Polynya · 2024-11-20T12:07:17Z

web/modules/custom/ecc_cct/README.md

@@ -0,0 +1,11 @@
+# Essex Co Co cookie consent statistics.


ok, what am I missing? I don't see it.

The abbreviation is confusing. Change to Essex County Council or ECC, like you have in other places.

Polynya · 2024-11-20T12:10:26Z

web/modules/custom/ecc_cct/js/ecc_cct.js

+        // several times per page load.
+        count++;
+        if (count === 2) {
+          console.log(response);


Remove the console logs or add a debug mode so we can control if they are written.

console.log removed, PR updated.

Polynya · 2024-11-20T12:11:43Z

web/modules/custom/ecc_cct/src/Controller/ConsentLog.php

+    ];
+
+    /** @var \Drupal\Core\Database\Connection $connection */
+    $connection = \Drupal::service('database');


This should use a content entity instead of directly accessing the database table. It would make CRUD much easier.

Nah. The idea is to make this as minimalistic as possible, with the least overhead. Entities come with overhead. The DB write here couldn't be simpler, and the data extraction is a single SQL query. Entities and all of Drupal's APIs would be needless expense. In fact, we only ever want to do create and read, never update nor delete. We're doing just enough, and no more.

OK. Don't call services like this. Use create() for dependency injection.

web/modules/custom/ecc_cct/ecc_cct.install

Polynya · 2024-11-21T10:42:06Z

web/modules/custom/ecc_cct/README.md

@@ -0,0 +1,11 @@
+# Essex Co Co cookie consent statistics.


The abbreviation is confusing. Change to Essex County Council or ECC, like you have in other places.

Polynya · 2024-11-21T10:42:45Z

web/modules/custom/ecc_cct/config/schema/ecc_cct.schema.yml

+  label: 'ECC Cookie Consent Tracker settings'
+  mapping:
+    enable:
+      type: string


Generate may be quick but it's no guarantee of good code.

Polynya · 2024-11-21T10:43:55Z

web/modules/custom/ecc_cct/js/ecc_cct.js

+
+      async function ecc_cct_log(choice) {
+        const response = await fetch('/ecc-cct/js/log/' + choice).catch(error => {
+          console.error('Error sending integer:', error);


Why do users need to see anything in the console?

Polynya · 2024-11-21T10:45:01Z

web/modules/custom/ecc_cct/src/Form/SettingsForm.php

+        0 => $this->t('Disabled'),
+        1 => $this->t('Enabled'),
+      ],
+      '#default_value' => $this->config('ecc_cct.settings')->get('enable'),


Add #config_target and you should be able to remove default_value and the submitForm method.

Polynya · 2024-11-21T10:54:23Z

web/modules/custom/ecc_cct/src/Controller/ConsentLog.php

+   */
+  public function log(int $choice, Request $request): Response {
+    // If this isn't turned on, bail.
+    if (!\Drupal::config('ecc_cct.settings')->get('enable')) {


It would probably be faster (one database read?) to use a State setting instead of config. Either way use the methods state() or config() in ControllerBase.

Polynya · 2024-11-21T11:21:09Z

web/modules/custom/ecc_cct/src/Controller/ConsentLog.php

+    ];
+
+    /** @var \Drupal\Core\Database\Connection $connection */
+    $connection = \Drupal::service('database');


OK. Don't call services like this. Use create() for dependency injection.

Polynya · 2024-11-21T11:24:57Z

web/modules/custom/ecc_cct/ecc_cct.module

+ * Implements hook_preprocess_html().
+ */
+function ecc_cct_preprocess_html(&$vars) {
+  if ($vars['root_path'] !== 'admin') {


It would be better to exclude the same paths as EU Cookie Compliance

Changes as follows:

using eu cookie settings for excluded paths

using dependency injection for db service access

using config() from controllerbase to access config

using #config_target instead of default_value

js is printing nothing to console now

changed config schema to use integer as integer is what's actually used (boolean makes radio button choke)

…orm.

…l/essex-gov-uk-drupal into 202411_consentratio_LP_135

Polynya · 2024-11-27T18:14:55Z

/start

github-actions · 2024-11-27T18:15:23Z

Starting Review App.

github-actions · 2024-11-27T18:22:32Z

Started Review App
https://portal---202411-consentratio-lp-135.livelyforest-609fad32.uksouth.azurecontainerapps.io/

anthonylindsay added 5 commits November 7, 2024 20:22

Custom module to track cookie consent ratio.

1946ebf

Enable ecc cct module

2f42aab

Add settings form to cookie consent tracker for enable/disable

b96db34

Enable consent choice tracking

3106319

Change settings form route and name

35113d9

Polynya reviewed Nov 20, 2024

View reviewed changes

Remove console.log from cookie choice tracker js

d41ba3d

Polynya reviewed Nov 21, 2024

View reviewed changes

anthonylindsay and others added 11 commits November 21, 2024 15:45

Swap abbreviation for full council name for clarity.

772d573

Change string to integer as an integer is actually used in settings f…

6679bdb

…orm.

Coding standards adjustments for install file.

721ea93

Remove all console output from cookie choice js

a2d8721

Use eu cookie module and get excluded paths from it.

3e1a290

Use dependency injection for accessing db services

b5362f9

use config_target for settings form to reduce code overhead

0bbe4e2

chore(): Drupal core 10.3.10

081f9ec

chore(): Security update for phpspreadsheet

66394cb

Merge branch 'release/drupal-10.3.10' of github.com:essexcountycounci…

e0c965d

…l/essex-gov-uk-drupal into 202411_consentratio_LP_135

fix(LP-135): CodeSniffer

e799c66

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

202411 consentratio lp 135 #495

202411 consentratio lp 135 #495

anthonylindsay commented Nov 11, 2024

Polynya Nov 20, 2024

anthonylindsay Nov 20, 2024

Polynya Nov 21, 2024

Polynya Nov 20, 2024

anthonylindsay Nov 20, 2024

Polynya Nov 21, 2024

Polynya Nov 20, 2024

anthonylindsay Nov 20, 2024

Polynya Nov 20, 2024

anthonylindsay Nov 20, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

Polynya Nov 21, 2024

anthonylindsay Nov 21, 2024

Polynya commented Nov 27, 2024

github-actions bot commented Nov 27, 2024

github-actions bot commented Nov 27, 2024

202411 consentratio lp 135 #495

Are you sure you want to change the base?

202411 consentratio lp 135 #495

Conversation

anthonylindsay commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Polynya commented Nov 27, 2024

github-actions bot commented Nov 27, 2024

github-actions bot commented Nov 27, 2024