AC-2574: Commerce Integrated Google Modules Upgrade - Added new module Google Gtag #35376

mahesh-singh-rajawat · 2022-04-26T15:12:22Z

Fetched all the Gtag changes only from #32623 and creates New module Google Gtag.

Description (*)

Fetched all the Gtag changes only from #32623 and creates New module Google Gtag.

Related Pull Requests

https://github.com/magento-commerce/magento2ee/pull/3297
https://github.com/magento-commerce/magento2-infrastructure/pull/1676

Fixed Issues (if relevant)

Google Universal Analytics will be EOL July 2023 #35204

Manual testing scenarios (*)

...
...

…e Google Gtag

m2-assistant · 2022-04-26T15:12:25Z

Hi @mahesh-singh-rajawat. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

mahesh-singh-rajawat · 2022-04-26T15:19:03Z

@magento run all tests

magento-automated-testing · 2022-04-26T15:19:08Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

sidolov

Add strict types to all methods where it's possible
Move all files into one module - GoogleGtag
Property name shouldn't contain an underscore and should be private where it's possible

sidolov · 2022-04-26T15:25:39Z

app/code/Magento/GoogleGtag/Block/Code.php

+    /**
+     * @var Data
+     */
+    protected $_googleGtagData;


Please, make property private and remove underscore from the name

Updated the variable scope and removed underscore

sidolov · 2022-04-26T15:26:08Z

app/code/Magento/GoogleGtag/Block/Code.php

+     *
+     * @return Data
+     */
+    public function getHelper()


Where this method is used?

Its getting use in https://github.com/magento/magento2/pull/35376/files#diff-4a750488ca4bf078ec74ee38c0317d7d43370b84066486a4ecc8b309a7b4d0e5R11

sidolov · 2022-04-26T15:26:34Z

app/code/Magento/GoogleGtag/Block/Ga.php

+    /**
+     * @var Data
+     */
+    protected $_googleGtagData = null;


Please, make property private and remove underscore from the name

Updated the variable scope and removed underscore

sidolov · 2022-04-26T15:26:38Z

app/code/Magento/GoogleGtag/Block/Ga.php

+    /**
+     * @var CollectionFactory
+     */
+    protected $_salesOrderCollection;


Please, make property private and remove underscore from the name

Updated the variable scope and removed underscore

sidolov · 2022-04-26T15:26:57Z

app/code/Magento/GoogleGtag/Block/Ga.php

+     * @param string $path
+     * @return mixed
+     */
+    public function getConfig($path)


Add strict types to all methods

This function was not used, i have removed this.

sidolov · 2022-04-26T15:27:31Z

app/code/Magento/GoogleGtag/Block/Ga.php

+    /**
+     * Return information about page for GA tracking
+     *
+     * @link https://developers.google.com/analytics/devguides/collection/gtagjs


Do these links are still relevant?

This link shows the use of gtag for page_view event, if it requires to find easily reference of using Gtag, we can keep it.

sidolov · 2022-04-26T15:28:00Z

app/code/Magento/GoogleGtag/Block/Head.php

+    /**
+     * @var Data
+     */
+    protected $googleGtagData;


Make property private

Updated the variable scope to private

sidolov · 2022-04-26T15:28:43Z

app/code/Magento/GoogleGtag/Helper/Data.php

+ *
+ * @api
+ */
+class Data extends AbstractHelper


I would rename the helper in to GtagConfiguration

Helper Data renamed to GtagConfiguration and respective files updated.

mahesh-singh-rajawat · 2022-04-27T12:24:26Z

@magento run all tests

magento-automated-testing · 2022-04-27T12:24:31Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-04-28T03:15:14Z

@magento run Functional Tests B2B,Functional Tests CE,Functional Tests EE

magento-automated-testing · 2022-04-28T03:15:22Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ishakhsuvarov · 2022-05-03T15:08:35Z

app/code/Magento/GoogleGtag/Helper/GtagConfiguration.php

+ *
+ * @api
+ */
+class GtagConfiguration extends AbstractHelper


What is the point of extending AbstractHelper? I would suggest to avoid extending layer supertypes, especially helpers, whenever it is possible.

Helper class removed and Model created to provide config values.

ishakhsuvarov · 2022-05-03T15:09:40Z

app/code/Magento/GoogleGtag/Helper/GtagConfiguration.php

+use Magento\Store\Model\Store;
+
+/**
+ * GoogleAnalytics data helper


Let's make a configuration model out of this. Helper is not really a recommended approach as there is absolutely no clear indication of its purpose and responsibility.

Helper class removed and Model created to provide config values.

ishakhsuvarov · 2022-05-03T15:10:00Z

app/code/Magento/GoogleGtag/Helper/GtagConfiguration.php

+ */
+declare(strict_types=1);
+
+namespace Magento\GoogleGtag\Helper;


I'd move it to Model namespace along with other refactoring.

Helper class removed and Model created to provide config values

ishakhsuvarov · 2022-05-03T15:10:27Z

app/code/Magento/GoogleGtag/Helper/GtagConfiguration.php

+    /**
+     * Config paths for using throughout the code
+     */
+    public const XML_PATH_ACTIVE = 'google/gtag/analytics4/active';


Do these constants need to be public?

(same question applies to all public constants below) We need a clear reason to make it public – otherwise should be private.

updated the scope of constants.

ishakhsuvarov · 2022-05-03T16:13:24Z

app/code/Magento/GoogleGtag/Block/Code.php

+ * Google Ads Code block
+ *
+ * @api
+ * @since 100.0.2


Why is it exactly 100.0.2?

This block removed now

ishakhsuvarov · 2022-05-04T17:29:49Z

app/code/Magento/GoogleGtag/Block/Ga.php

+     * @link https://developers.google.com/gtagjs/reference/event#purchase
+     *
+     * @return array
+     * @since 100.2.0


Why is this specific method is marked with @since and how is it exactly 100.2.0?

Removed this code, as its a new Module

ishakhsuvarov · 2022-05-04T17:31:32Z

app/code/Magento/GoogleGtag/Block/Ga.php

+            return $result;
+        }
+
+        $collection = $this->salesOrderCollection->create();


It could possibly make sense to use an api \Magento\Sales\Api\OrderRepositoryInterface::getList

Updated to get order list via Order \Magento\Sales\Api\OrderRepositoryInterface::getList

ishakhsuvarov · 2022-05-04T17:32:27Z

app/code/Magento/GoogleGtag/Block/Head.php

+ *
+ * @api
+ */
+class Head extends Template


Can this block be replaced by a view model?

See reference above.

Moved this block to View model

ishakhsuvarov · 2022-05-04T17:34:48Z

app/code/Magento/GoogleGtag/Helper/GtagConfiguration.php

+     * @param float $numberString
+     * @return Float
+     */
+    public function formatToDec($numberString)


Can this just be used in place? Do we really need a public method (say @api) for this operation located in the arbitrary helper?

Removed it and directly use this function in Block

ishakhsuvarov · 2022-05-04T17:38:44Z

app/code/Magento/GoogleGtag/view/frontend/templates/ga.phtml

+            "isCookieRestrictionModeEnabled": <?= (int)$block->isCookieRestrictionModeEnabled() ?>,
+            "currentWebsite": <?= (int)$block->getCurrentWebsiteId() ?>,
+            "cookieName": "<?= /* @noEscape */ \Magento\Cookie\Helper\Cookie::IS_USER_ALLOWED_SAVE_COOKIE ?>",
+            "pageTrackingData": <?= /* @noEscape */ json_encode($block->getPageTrackingData($measurementId)) ?>,


I would suggest serializing it in the block (or view model) using an api like \Magento\Framework\Serialize\SerializerInterface

Moved code to Ga Block

…lper

mahesh-singh-rajawat · 2022-05-06T11:39:08Z

@magento run all tests

magento-automated-testing · 2022-05-06T11:39:12Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

magento-automated-testing · 2022-05-20T01:51:27Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-05-20T14:13:49Z

@magento run all tests

magento-automated-testing · 2022-05-20T14:13:58Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

…into gtag

…to gtag

ishakhsuvarov · 2022-05-23T14:23:08Z

@magento run all tests

magento-automated-testing · 2022-05-23T14:23:16Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

glo41183 · 2022-05-24T12:44:49Z

@magento run all tests

magento-automated-testing · 2022-05-24T12:44:59Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-05-24T15:22:15Z

@magento run all tests

magento-automated-testing · 2022-05-24T15:22:26Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-05-24T17:19:50Z

@magento run Functional Tests B2B,Functional Tests EE,Integration Tests

magento-automated-testing · 2022-05-24T17:19:58Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-05-25T06:22:30Z

@magento run all tests

magento-automated-testing · 2022-05-25T06:22:43Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mahesh-singh-rajawat · 2022-05-25T13:45:12Z

@magento run Functional Tests B2B,Functional Tests EE,Database Compare,Static Tests

mahesh-singh-rajawat · 2022-05-25T13:47:55Z

@magento run all tests

magento-automated-testing · 2022-05-25T13:48:03Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ryanisn · 2022-05-27T13:12:19Z

When do we expect this released? Can this be released as official marketplace extension so we can install it on earlier 2.4.x versions?
Merchants would want to install this update ASAP to keep more historical data, google will only keep 6mo data on GA3

AC-2574: Commerce Integrated Google Modules Upgrade - Added new modul…

3b0f2a3

…e Google Gtag

sidolov previously requested changes Apr 26, 2022

View reviewed changes

m2-community-project bot assigned sidolov Apr 26, 2022

m2-community-project bot added this to Changes Requested in Pull Requests Dashboard Apr 26, 2022

m2-community-project bot added the Progress: needs update label Apr 26, 2022

AC-2574 - updated variables scope and method return strict type

7b41128

m2-community-project bot moved this from Changes Requested to Review in Progress in Pull Requests Dashboard Apr 27, 2022

m2-community-project bot added Progress: review and removed Progress: needs update labels Apr 27, 2022

mahesh-singh-rajawat requested a review from sidolov April 27, 2022 12:31

ishakhsuvarov requested changes May 4, 2022

View reviewed changes

m2-community-project bot assigned ishakhsuvarov May 4, 2022

m2-community-project bot moved this from Review in Progress to Changes Requested in Pull Requests Dashboard May 4, 2022

m2-community-project bot added Progress: needs update and removed Progress: review labels May 4, 2022

sdzhepa added the Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. label May 5, 2022

m2-community-project bot added this to Changes Requested in High Priority Pull Requests Dashboard May 5, 2022

m2-community-project bot removed this from Changes Requested in Pull Requests Dashboard May 5, 2022

AC-2574: Moved Block to view model, created config Model to remove He…

6e63230

…lper

m2-community-project bot moved this from Changes Requested to Review in Progress in High Priority Pull Requests Dashboard May 6, 2022

m2-community-project bot removed the Progress: needs update label May 6, 2022

AC-2574 moved inline js in separate file

8eb0752

m2-community-project bot moved this from Changes Requested to Review in Progress in High Priority Pull Requests Dashboard May 20, 2022

m2-community-project bot added Progress: review and removed Progress: needs update labels May 20, 2022

ishakhsuvarov added 2 commits May 23, 2022 09:21

Merge branch '2.4-develop' of github.com:magento-commerce/magento2ce …

6b96b31

…into gtag

Merge branch 'AC-2574' of github.com:mahesh-singh-rajawat/magento2 in…

598ae1c

…to gtag

Merge branch '2.4-develop' of github.com:magento/magento2 into AC-2574

21aa593

Merge branch '2.4-develop' into AC-2574

4c56a3c

magento-devops-reposync-svc merged commit 9b91ff9 into magento:2.4-develop May 27, 2022

This was referenced May 28, 2022

Feature/gtag update #32623

Closed

Google Universal Analytics will be EOL July 2023 #35204

Closed

ishakhsuvarov moved this from Review in Progress to Recently Merged in High Priority Pull Requests Dashboard Jun 10, 2022

hostep mentioned this pull request Feb 11, 2023

Google Universal Analytics becoming obsolete #36843

Closed

5 tasks

AC-2574: Commerce Integrated Google Modules Upgrade - Added new module Google Gtag #35376

AC-2574: Commerce Integrated Google Modules Upgrade - Added new module Google Gtag #35376

Conversation

mahesh-singh-rajawat commented Apr 26, 2022 • edited by ishakhsuvarov

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

m2-assistant bot commented Apr 26, 2022

mahesh-singh-rajawat commented Apr 26, 2022

magento-automated-testing bot commented Apr 26, 2022

sidolov left a comment • edited

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

mahesh-singh-rajawat Apr 27, 2022 • edited

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

mahesh-singh-rajawat commented Apr 27, 2022

magento-automated-testing bot commented Apr 27, 2022

mahesh-singh-rajawat commented Apr 28, 2022

magento-automated-testing bot commented Apr 28, 2022

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

mahesh-singh-rajawat May 6, 2022 • edited

Choose a reason for hiding this comment

mahesh-singh-rajawat commented May 6, 2022

magento-automated-testing bot commented May 6, 2022

magento-automated-testing bot commented May 20, 2022

mahesh-singh-rajawat commented May 20, 2022

magento-automated-testing bot commented May 20, 2022

ishakhsuvarov commented May 23, 2022

magento-automated-testing bot commented May 23, 2022

glo41183 commented May 24, 2022

magento-automated-testing bot commented May 24, 2022

mahesh-singh-rajawat commented May 24, 2022

magento-automated-testing bot commented May 24, 2022

mahesh-singh-rajawat commented May 24, 2022

magento-automated-testing bot commented May 24, 2022

mahesh-singh-rajawat commented May 25, 2022

magento-automated-testing bot commented May 25, 2022

mahesh-singh-rajawat commented May 25, 2022 • edited

mahesh-singh-rajawat commented May 25, 2022

magento-automated-testing bot commented May 25, 2022

ryanisn commented May 27, 2022

mahesh-singh-rajawat commented Apr 26, 2022 •

edited by ishakhsuvarov

sidolov left a comment •

edited

mahesh-singh-rajawat Apr 27, 2022 •

edited

mahesh-singh-rajawat May 6, 2022 •

edited

mahesh-singh-rajawat commented May 25, 2022 •

edited