Skip to content

check if order data is available to incl ec #15765

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

Merged
merged 1 commit into from
Jun 5, 2018
Merged

check if order data is available to incl ec #15765

merged 1 commit into from
Jun 5, 2018

Conversation

torhoehn
Copy link
Contributor

@torhoehn torhoehn commented Jun 4, 2018

Description

After merging #13034 ecommerce tracking wasn't working, because it's not possible to call length on an object (ordersTrackingData).

Fixed Issues (if relevant)

  1. Google analytics pageview being triggered twice #12221: Google analytics pageview being triggered twice (see comment Google analytics pageview being triggered twice #12221 (comment))

Manual testing scenarios

  1. Created a Google Analytics test account
  2. Order something
  3. Order will appear in Google Analytics (Conversion -> E-Commerce) or see console with running Google Analytics Debugger (https://chrome.google.com/webstore/detail/google-analytics-debugger/jnkmfdileelhofjcijamephohjechhna). Transfered data will contain product specific data.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jun 4, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

@torhoehn thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team
Copy link
Contributor

Hi @torhoehn. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.6 release.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

@torhoehn torhoehn deleted the ga-ec-tracking-fix branch June 5, 2018 05:46
@torhoehn torhoehn restored the ga-ec-tracking-fix branch June 5, 2018 05:47
@torhoehn torhoehn deleted the ga-ec-tracking-fix branch June 5, 2018 11:52
@ihor-sviziev ihor-sviziev added partners-contribution Pull Request is created by Magento Partner and removed partner-contribution labels Jun 5, 2018
@sashas777
Copy link
Contributor

If someone need temporary fix for the version 2.2.4 with the assumption that it would be fixed in the newest release. Feel free to use it: https://github.com/sashas777/magento224-issue14951

This made based on PR by @torhoehn - all thanks to him

@peterjaap
Copy link
Contributor

@sashas777 why does that work only with 2.2.4 and not with 2.2.3?

@sashas777
Copy link
Contributor

@peterjaap I applied fix based on version 2.2.4 and didn't test for any other versions. This is only the reason

@asadyar
Copy link

asadyar commented Oct 1, 2018

Google Analytics tracking is still missing in Magento 2.2.6 composer update from 2.2.5

Is there any deadline or temporary fix available for this.

Thanks,

@torhoehn
Copy link
Contributor Author

torhoehn commented Oct 1, 2018

@asadyar The change from this PR is present in 2.2.6.

@asadyar
Copy link

asadyar commented Oct 1, 2018

Thanks torhoehn, I clear and flush Magento 2 cache, I can see its triggering the Google Analytics event on success page.

Thanks,
Best Regards,
Asad

@asadyar
Copy link

asadyar commented Oct 1, 2018 via email

@nickpiro
Copy link

Should I be fine making this update in 2.2.5 instead up updating to 2.2.6?

@torhoehn
Copy link
Contributor Author

@nickpiro Yes, it'll work for 2.2.5, too. E.g. copy the .js-file to your theme and make the change.

@nickpiro
Copy link

@torhoehn I updated the one in vendor with this file and it still does not work! :/

@torhoehn
Copy link
Contributor Author

@nickpiro Feel free to contact me on Slack: https://tinyurl.com/engcom-slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants