Add analytics to npq #1917
Add analytics to npq #1917
Conversation
slawosz
force-pushed
the
2061-dfe-analytics
branch
from
7b00314 to
03b1fb9
Compare
config/initializers/dfe_analytics.rb
Outdated
| # config.user_identifier = proc { |user| user&.id | ||
|
|
||
| # if part of any model name is migration, it wont be loaded | ||
| config.excluded_models_proc = proc { |x| x.to_s =~ /Migration::/ } |
There was a problem hiding this comment.
Can the migration things not be added to the blocklist for now?
There was a problem hiding this comment.
The migration needs to be added, otherwise plugin will not work.
Currently we have models like Statement and Migration::Statement, which are pointing to 2 databases, but both are using statements table. The dfe-analytics operates on tables names so it gets confused by 2 tables that have the same name but are using 2 different schemas.
Excluding Migration:: tables does not give much difference - all important data will be exported anyway.
|
Looks good, I think you just need to regenerate the blocklist. |
| # A proc which returns true or false depending on whether you want to | ||
| # enable analytics. You might want to hook this up to a feature flag or | ||
| # environment variable. | ||
| config.enable_analytics = proc { Rails.env.staging? } |
There was a problem hiding this comment.
should we add a feature flag instead so we can enable it in the envs we want incase?
config/initializers/dfe_analytics.rb
Outdated
| # The environment we’re running in. This value will be attached | ||
| # to all events we send to BigQuery. | ||
| # | ||
| config.environment = ENV.fetch('RAILS_ENV', 'development') |
There was a problem hiding this comment.
not sure we want to enable this?
There was a problem hiding this comment.
Is there any spec to add maybe?
Also It'd be a nice idea to have a look at the yml files in ECF as a base to generate the lists here?
https://github.com/DFE-Digital/early-careers-framework/blob/main/config/analytics_blocklist.yml
and
https://github.com/DFE-Digital/early-careers-framework/blob/main/config/analytics_pii.yml
config/initializers/dfe_analytics.rb
Outdated
| # environment variable. | ||
| config.enable_analytics = proc { Rails.env.staging? } | ||
|
|
||
|
|
There was a problem hiding this comment.
We also have this job enabled in ECF.. does it make any difference if we also add here?
https://github.com/DFE-Digital/dfe-analytics?tab=readme-ov-file#entity-table-check-job
slawosz
force-pushed
the
2061-dfe-analytics
branch
from
03b1fb9 to
6116ce7
Compare
slawosz
force-pushed
the
2061-dfe-analytics
branch
from
6116ce7 to
a63edaa
Compare
|
Review app deployed to https://npq-registration-review-1917-web.test.teacherservices.cloud/ |
slawosz
force-pushed
the
2061-dfe-analytics
branch
from
a63edaa to
b4f2e6b
Compare
slawosz
force-pushed
the
2061-dfe-analytics
branch
from
b4f2e6b to
1f11e4c
Compare
|
This PR is introducing def-analytics to
NPQ.Currently analytics will be send only to
stagingenvironment to test integration.There is extra PR to be merge to allow excluding certain models, for the time being
npqwill be using fork: DFE-Digital/dfe-analytics#169