Skip to content
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

feat: add support to GA4 DS-577 #169

Merged
merged 2 commits into from
Jul 21, 2023
Merged

feat: add support to GA4 DS-577 #169

merged 2 commits into from
Jul 21, 2023

Conversation

MaferMazu
Copy link
Contributor

@MaferMazu MaferMazu commented Jul 17, 2023

Allow tracking data with GA4. To do so, we need to set: GOOGLE_ANALYTICS_4_ID.

Important note: This is still working as usual for the clients (tenants).

The inspiration for these changes comes from openedx/edx-platform#32032

How to test it

  • Have a tutor env with eox-theming and this theme.
    • The easy way to do it is with distro and with this configs:
DEV_PROJECT_NAME: ga4_dev
LOCAL_PROJECT_NAME: ga4_local
DISTRO_THEMES_NAME:
  - bragi
DISTRO_THEME_DIRS:
  - "/openedx/themes/ednx-saas-themes/edx-platform/bragi-generator"
  - "/openedx/themes/ednx-saas-themes/edx-platform/bragi-children"
  - "/openedx/themes/ednx-saas-themes/edx-platform"
DISTRO_THEMES_ROOT: "/openedx/themes"
DISTRO_THEMES:
  - name: ednx-saas-themes
    repo: ednx-saas-themes
    version: mfmz/ga4
    domain: github.com
    protocol: ssh
    path: eduNEXT
DISTRO_EOX_THEMING_DPKG:
  index: git
  name: eox-theming
  repo: eox-theming
  domain: github.com
  path: eduNEXT
  protocol: https
  private: false
  variables:
    development:
      EOX_THEMING_CONFIG_SOURCES: [
        "from_eox_tenant_microsite_v2",
        "from_django_settings"
      ]
    production:
      EOX_THEMING_CONFIG_SOURCES: [
        "from_eox_tenant_microsite_v2",
        "from_django_settings"
      ]
  version: v5.0.0
DISTRO_EOX_TENANT_DPKG:
  index: git
  name: eox-tenant
  repo: eox-tenant
  domain: github.com
  path: eduNEXT
  protocol: https
  private: false
  variables:
    development:
      EOX_TENANT_USERS_BACKEND: eox_tenant.edxapp_wrapper.backends.users_l_v1
      EOX_TENANT_LOAD_PERMISSIONS: false
    production:
      EOX_TENANT_USERS_BACKEND: eox_tenant.edxapp_wrapper.backends.users_l_v1
      EOX_TENANT_LOAD_PERMISSIONS: false
  version: v8.0.0
OPENEDX_EXTRA_SETTINGS:
  cms_env: [
    USE_EOX_TENANT: true
  ]
  lms_env: [
    USE_EOX_TENANT: true,
    ENABLE_EOX_THEMING_DERIVE_WORKAROUND: true
  ]

(Run: tutor config save, tutor distro enable-themes, and tutor images build openedx)

  • In env/apps/openedx/config/lms.env.yml add some string in GOOGLE_ANALYTICS_4_ID and FEATURES["EDNX_ENABLE_GOOGLE_ANALYTICS"]=True
  • Run the platform and not fail.

For deep test:

  • Add <p>${static.get_value("GOOGLE_ANALYTICS_4_ID", settings.GOOGLE_ANALYTICS_4_ID)}</p> in env/build/openedx/themes/ednx-saas-themes/edx-platform/bragi/lms/templates/google_analytics.html
  • Reload with: tutor dev exec lms reload-uwsgi
  • Search for the string you put in GOOGLE_ANALYTICS_4_ID in elements, in inspect, in your browser.
    Screenshot from 2023-07-18 21-44-46

@MaferMazu MaferMazu changed the title feat: add support to GA4 feat: add support to GA4 DS-577 Jul 18, 2023
Copy link
Contributor

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

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

I tried adding EDNX_ENABLE_GOOGLE_ANALYTICS y GOOGLE_ANALYTICS_4_ID in the lms.env.yml to test the conditional % if ga_4_id: when is added or skipped the a GA4 ID and this happend:

tutor_dev-lms-1                        |     ga_4_id = static.get_value("GOOGLE_ANALYTICS_4_ID", settings.GOOGLE_ANALYTICS_4_ID)
tutor_dev-lms-1                        | AttributeError: 'Undefined' object has no attribute 'get_value'
tutor_dev-lms-1                        | Internal Server Error: /dashboard

help!

@MaferMazu
Copy link
Contributor Author

My bad, @bra-i-am. I needed to define the static and realized that I needed to import the js_escape_string. Can you review it again, please?

Copy link
Contributor

@bra-i-am bra-i-am 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 @MaferMazu, I thought I was missing some configuration.

Now looks good to me and is working as expected 👌

image

Copy link
Member

@JuanDavidBuitrago JuanDavidBuitrago left a comment

Choose a reason for hiding this comment

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

It is working as expected!
image

@MaferMazu MaferMazu merged commit e7f864f into master Jul 21, 2023
MaferMazu added a commit that referenced this pull request Jul 21, 2023
* feat: add support to GA4

* fix: define static and import js_escaped_string
<% ga_4_id = static.get_value("GOOGLE_ANALYTICS_4_ID", settings.GOOGLE_ANALYTICS_4_ID) %>
% if ga_4_id:
ga('create', '${ga_4_id | n, js_escaped_string}', 'auto', {'name': 'edunext', 'alwaysSendReferrer': true});
ga('edunext.require', 'displayfeatures');
Copy link
Member

Choose a reason for hiding this comment

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

Creo que no era necesario mantener el nombre de edunext aquí. Eso era particularmente útil cuando teniamos 2 cuentas de analytics para reportar. Si solo vamos a mantener una ya no necesita eso.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dejé las dos cuentas de analytics @felipemontoya, realmente lo que se hizo fue simplemente que el tracking code no estuviera quemado. Y tome esa decisión porque en el equipo de marketing me dijero que lo están usando.

Copy link
Member

Choose a reason for hiding this comment

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

El cuento es que en esta linea:

ga('create', '${ga_4_id | n, js_escaped_string}', 'auto', {'name': 'edunext', 'alwaysSendReferrer': true})

Lo defines con el nombre edunext. Luego en las otras dos lo usas con ese nombre. Puedes ponerle otro nombre, como analytics o cualquier otra cosa. Esto para que cuando un cliente usa su propio código no parezca que son nuestras analyticas cuando no lo son.

ErickMurillo pushed a commit that referenced this pull request Aug 1, 2023
* feat: add support to GA4

* fix: define static and import js_escaped_string
ErickMurillo added a commit that referenced this pull request Aug 2, 2023
DeimerM pushed a commit that referenced this pull request Oct 22, 2024
* feat: add support to GA4

* fix: define static and import js_escaped_string
DeimerM pushed a commit that referenced this pull request Oct 22, 2024
* feat: add support to GA4

* fix: define static and import js_escaped_string
DeimerM added a commit that referenced this pull request Oct 22, 2024
* feat: add support to GA4

* fix: define static and import js_escaped_string

Co-authored-by: María Fernanda Magallanes <35668326+MaferMazu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants