CartoDB / cartodb

Location Intelligence & Data Visualization tool
http://carto.com
BSD 3-Clause "New" or "Revised" License
2.74k stars 653 forks source link

Google Tag Manager is missing in some pages #14029

Closed alonsogarciapablo closed 6 years ago

alonsogarciapablo commented 6 years ago

I setup Google Tag Manager in https://github.com/CartoDB/cartodb/pull/13946.

It was working perfectly until May 25th (aka GDPR birthday) and is not present in some templates anymore. For example in /dashboard:

Some other pages where is no longer present:

cc: @ivanmalagon @javitonino

javitonino commented 6 years ago

Dashboard has undergone a major revamp (switch to static assets) so it's expected that it isn't there if you didn't warn frontend.

About the /upgrade path (central), I don't think it ever worked. That is handled by the upgrade layout and in the commit history there is no mention of GTM even being added.

alonsogarciapablo commented 6 years ago

Thanks @javitonino

About the /upgrade path (central), I don't think it ever worked. That is handled by the upgrade layout and in the commit history there is no mention of GTM even being added.

See https://github.com/CartoDB/cartodb-central/pull/2315

alonsogarciapablo commented 6 years ago

cc: @luisbosque

jesusbotella commented 6 years ago

Hey! I was just going to take care of this but I need to know which script I have to insert on Profile and Account page, because as far as I see there are two variants (one with script, and another one without the script).

Apart from that, I see the created_at property that we need is not exposed in /api/v3/me endpoint, so there's some backend job needed.

alonsogarciapablo commented 6 years ago

Thanks @jesusbotella. We should include both the <script> and <noscript> tags for the primary container. Basically:

...
<head>
  ...
  <%= insert_google_tag_manager('primary') %>
</head>
<body>
  <%= insert_google_tag_manager_no_script('primary') %>
  ...
</body>

Apart from that, I see the created_at property that we need is not exposed in /api/v3/me endpoint, so there's some backend job needed.

Is current_user available? If so, is there other property related to the date/time when the user signed up?

jesusbotella commented 6 years ago

Yes, sorry, I forgot to mention it. current_user is available, but I don't see any other property similar to created_at. :(

Here I paste all the properties that we have available there:

[
  "id",
  "email",
  "name",
  "last_name",
  "username",
  "account_type",
  "account_type_display_name",
  "table_quota",
  "table_count",
  "viewer",
  "industry",
  "company",
  "phone",
  "job_role",
  "org_admin",
  "public_visualization_count",
  "owned_visualization_count",
  "all_visualization_count",
  "visualization_count",
  "failed_import_count",
  "success_import_count",
  "import_count",
  "last_visualization_created_at",
  "quota_in_bytes",
  "db_size_in_bytes",
  "db_size_in_megabytes",
  "remaining_table_quota",
  "remaining_byte_quota",
  "api_calls",
  "api_calls_quota",
  "api_calls_block_price",
  "geocoding",
  "here_isolines",
  "mapzen_routing",
  "geocoder_provider",
  "isolines_provider",
  "routing_provider",
  "obs_snapshot",
  "obs_general",
  "twitter",
  "salesforce",
  "mailchimp",
  "billing_period",
  "api_key",
  "layers",
  "trial_ends_at",
  "upgraded_at",
  "show_trial_reminder",
  "show_upgraded_message",
  "show_builder_activated_message",
  "actions",
  "limits",
  "notification",
  "avatar_url",
  "feature_flags",
  "base_url",
  "needs_password_confirmation",
  "description",
  "website",
  "twitter_username",
  "disqus_shortname",
  "available_for_hire",
  "location",
  "organization",
  "groups",
  "mobile_apps"
]
alonsogarciapablo commented 6 years ago

Yes, I don't see anything like created_at. Is this something we can add to that endpoint?

(cc: @javitonino)

javitonino commented 6 years ago

It should not be a problem to add the creation date. Feel free to add it to the user presenter & decorator.

Also, please note that this is not the actual signup date, but the creation date at the cloud, which is normally the same, but not always. The signup date is not available at the cloud, and this should be a good enough approximation, but just wanted to mention it for awareness.

alonsogarciapablo commented 6 years ago

Thanks @javitonino!

☝️ these two, right?

alonsogarciapablo commented 6 years ago

@jesusbotella do you feel 💪 about adding created_at to those?

jesusbotella commented 6 years ago

Let me try, I'll do my best 😂

rubenmoya commented 6 years ago

Can we close this?

jesusbotella commented 6 years ago

Yes!

On Wed, Jun 13, 2018 at 9:29 AM Rubén Moya notifications@github.com wrote:

Can we close this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CartoDB/cartodb/issues/14029#issuecomment-396841746, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHp8EioxR-dAQqtgeh7dJypHJ1H6Zv9ks5t8L9LgaJpZM4UWlR_ .

-- Jesús Botella Blanco Front-End Engineer www.carto.com