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

(#107) Update to modern analytics. #108

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Conversation

blairlearn
Copy link
Contributor

Closes #107

@blairlearn blairlearn changed the base branch from develop to release/v2.0.0 November 14, 2024 21:56
Copy link
Member

@bryanpizzillo bryanpizzillo left a comment

Choose a reason for hiding this comment

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

@blairlearn can you please check if you still need jquery? It is a really old version in use and should either be updated or removed. It looked like the jQuery was mostly for the analytics functions.

Also please remove the JS from the project if it is no longer being loaded. No need to confuse yourself in 6 months.

@blairlearn blairlearn force-pushed the ticket/107-analytics branch 2 times, most recently from 8ebad9a to a71e47d Compare November 18, 2024 16:38
@blairlearn
Copy link
Contributor Author

Also please remove the JS from the project if it is no longer being loaded. No need to confuse yourself in 6 months.

This will need to be a task for three months after the release. Otherwise, any pre-existing, not-yet-expired pages in the system will break.

- Leave BasicCTSPrintPage.js and rewrite rules for legacy JS files
  in order to support unexpired legacy pages.  These will need to be
  removed at a later date.
- Remove reference to JQuery (used for old analytics support).
- Remove legacy print-legacy.txt

Closes #107
@welshja
Copy link

welshja commented Nov 20, 2024

Changes looking good in testing. Some environment related hiccups, but we will re-review as this moves closer to production.

Copy link

@welshja welshja left a comment

Choose a reason for hiding this comment

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

Looks good to me

@blairlearn blairlearn dismissed bryanpizzillo’s stale review November 25, 2024 18:51

This has been addressed.

@blairlearn blairlearn merged commit 92e44fb into release/v2.0.0 Nov 25, 2024
2 checks passed
@blairlearn blairlearn deleted the ticket/107-analytics branch November 25, 2024 18:51
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.

Analytics changes to update implementation to current standards
3 participants