Cookie consent shows analytics usage also when GA is disabled#1816
Cookie consent shows analytics usage also when GA is disabled#1816tdonohue merged 12 commits intoDSpace:mainfrom
Conversation
# Conflicts: # src/app/shared/cookies/browser-klaro.service.ts
ref:
- refactored property loading and mapping
- refactored klaro initialisation
fix:
- now initialisation waits for google-analytics api keys.
… CST-6565 � Conflicts: � src/app/shared/cookies/browser-klaro.service.ts
ybnd
left a comment
There was a problem hiding this comment.
Functionally everything looks good to me, I've mainly listed some refactoring suggestions
|
Hi @ybnd, Firstly, thank you for the review. I'm sorry but I have some doubt about your review, could you clarify them? I left some comment above. Thank you! |
Test:
- New tests for the use case of filtering analytics configuration;
- Enhanced initialization of service with new google analytics service mock.
|
Hi @ybnd, refactored as you told me, and added some tests for the google analytics case. Thank you! |
ybnd
left a comment
There was a problem hiding this comment.
Thanks for the refactor!
I've added a few more remarks for the specs
|
Hi @ybnd, Thank You for the review, I refactored and cleaned the specs as You told me. |
ybnd
left a comment
There was a problem hiding this comment.
Thanks, the specs look good now!
tdonohue
left a comment
There was a problem hiding this comment.
👍 Thanks @vins01-4science ! The code looks good. I also tested this locally and verified that the Google Analytics section of the Klaro popup only appears when google.analytics.key is set on the backend.
Task/dspace cris 2023 02 x/DSC-1745 Approved-by: Andrea Barbasso
References
Description
This PR hides the analytics accordion when the api-key is not configured or is left blank on the server-side
Instructions for Reviewers
You can check this PR by changing the property field
google.analytics.keyinside thedspace.cfgconfiguration file.The expected behaviour for
google.analytics.keyvalue is:Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
yarn run lintpackage.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.