Conversation
Bumps the bundler group with 1 update in the / directory: [net-imap](https://github.com/ruby/net-imap). Updates `net-imap` from 0.6.4 to 0.6.4.1 - [Release notes](https://github.com/ruby/net-imap/releases) - [Commits](ruby/net-imap@v0.6.4...v0.6.4.1) --- updated-dependencies: - dependency-name: net-imap dependency-version: 0.6.4.1 dependency-type: indirect dependency-group: bundler ... Signed-off-by: dependabot[bot] <support@github.com>
- Included Trackable module in ApplicationController for tracking purposes. - Added new analytics jobs in recurring.yml for hourly, daily rollup, and pruning. - Updated queue.yml to configure analytics queue with specific threading and polling settings. - Defined route for analytics dashboard in routes.rb. - Enhanced layout views to include links to the analytics dashboard for authorized users. - Added Chart.js to importmap for data visualization support.
- Created tables for analytics_daily_rollups and analytics_hourly_rollups with relevant columns for tracking user interactions. - Added sequences for both tables to manage primary key generation. - Established primary key constraints and unique indexes to ensure data integrity and optimize query performance. - Included additional table for page_views with corresponding sequence and constraints for comprehensive analytics tracking.
- Implemented a refresh action in the AnalyticsDashboardController to trigger rollup jobs synchronously, allowing immediate chart updates. - Updated the analytics dashboard view to include a refresh button and a notice for stale data. - Modified the analytics policy to authorize the refresh action for admin users. - Adjusted routes to include a POST endpoint for refreshing analytics data. - Enhanced the PageView model with live summary and top pages methods to provide real-time analytics data. - Improved data handling in the controller to ensure top pages and summary stats reflect current data without waiting for rollup jobs.
- Modified the properties layer in tailwind.css to enhance support for various CSS features. - Updated theme and base layers to ensure consistent styling and improved layout handling. - Adjusted spacing and color variables for better design flexibility and responsiveness.
- Converted analytics dashboard views from HAML to ERB format for consistency. - Added new partials for stat cards and building rows to enhance modularity. - Updated Gemfile to remove HAML-related gems and dependencies. - Adjusted importmap to include Chart.js for improved data visualization. - Enhanced the .gitignore to exclude .OLD files and HAML files.
- Changed the schedule format for the AnalyticsRollupJob from "every week on Sunday at 02:30" to "at 2:30am every Sunday" for improved clarity and consistency.
…egration - Added a test to verify that hourly chart data is correctly scoped to the selected time range. - Updated the analytics chart controller to handle Turbo caching by destroying the chart before caching. - Modified the analytics dashboard view to prevent Turbo from restoring cached canvas data, ensuring fresh data is displayed on navigation.
…vention - Added a new test to verify that the selected time range is reflected on the analytics dashboard and that caching is disabled. - Updated the analytics dashboard view to display the selected time range and prevent browser caching. - Modified the analytics dashboard controller to include a method for setting cache control headers.
…ashboard - Added a new test to verify the use of a Stimulus controller for handling time range changes instead of inline JavaScript. - Updated the analytics dashboard view to utilize data attributes for the Stimulus controller, enhancing maintainability and separation of concerns. - Registered the new AnalyticsRangeController in the JavaScript controllers index.
Bumps the bundler group with 1 update in the / directory: [nokogiri](https://github.com/sparklemotion/nokogiri). Updates `nokogiri` from 1.19.3 to 1.19.4 - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.19.3...v1.19.4) --- updated-dependencies: - dependency-name: nokogiri dependency-version: 1.19.4 dependency-type: direct:production dependency-group: bundler ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: rsmokeUM <rsmoke@umich.edu>
Bumps the bundler group with 2 updates in the / directory: [concurrent-ruby](https://github.com/ruby-concurrency/concurrent-ruby) and [faraday](https://github.com/lostisland/faraday). Updates `concurrent-ruby` from 1.3.6 to 1.3.7 - [Release notes](https://github.com/ruby-concurrency/concurrent-ruby/releases) - [Changelog](https://github.com/ruby-concurrency/concurrent-ruby/blob/master/CHANGELOG.md) - [Commits](ruby-concurrency/concurrent-ruby@v1.3.6...v1.3.7) Updates `faraday` from 2.14.2 to 2.14.3 - [Release notes](https://github.com/lostisland/faraday/releases) - [Changelog](https://github.com/lostisland/faraday/blob/main/CHANGELOG.md) - [Commits](lostisland/faraday@v2.14.2...v2.14.3) --- updated-dependencies: - dependency-name: concurrent-ruby dependency-version: 1.3.7 dependency-type: indirect dependency-group: bundler - dependency-name: faraday dependency-version: 2.14.3 dependency-type: indirect dependency-group: bundler ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the analytics dashboard’s client-side behavior and maintainability by moving range selection to Stimulus, hardening Chart.js lifecycle handling under Turbo navigation, and ensuring analytics pages aren’t browser-cached. It also updates dependencies and applies small cleanups in DB structure dumps and recurring job scheduling.
Changes:
- Replace the time-range
onchangeinline handler with a newanalytics-rangeStimulus controller and update the analytics dashboard view accordingly. - Improve the
analytics-chartStimulus controller to destroy Chart.js instances onturbo:before-cacheand on disconnect; update chart canvases to be Turbo-temporary with range-specific ids. - Disable browser caching for the analytics index action; add request specs and bump/update supporting dependencies and minor config/SQL dump cleanups.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/analytics_dashboard_spec.rb | Adds request coverage for range-scoped charts, cache headers, and Stimulus wiring. |
| Gemfile | Bumps nokogiri constraint. |
| Gemfile.lock | Updates lockfile to newer dependency versions (including nokogiri). |
| db/structure.sql | Removes redundant schema header comment block. |
| db/queue_structure.sql | Removes redundant schema header comment block. |
| db/cable_structure.sql | Removes redundant schema header comment block. |
| config/recurring.yml | Adjusts schedule string formatting for the analytics prune job. |
| app/views/analytics_dashboard/index.html.erb | Updates range selector markup and adds range-specific chart canvas ids + Turbo temporary attributes. |
| app/models/analytics_hourly_rollup.rb | Uses all.group(...) for clearer scoped aggregation behavior. |
| app/models/analytics_daily_rollup.rb | Uses all.group(...) for clearer scoped aggregation behavior. |
| app/javascript/controllers/index.js | Registers the new analytics-range controller. |
| app/javascript/controllers/analytics_range_controller.js | Implements range-change navigation via Stimulus. |
| app/javascript/controllers/analytics_chart_controller.js | Adds proper Chart.js teardown for Turbo caching and disconnect. |
| app/controllers/analytics_dashboard_controller.rb | Adds response headers to prevent browser caching on the index action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Removed the inline data attribute for the analytics range URL in the dashboard view. - Updated the AnalyticsRangeController to construct the URL dynamically using the current window location, improving maintainability and flexibility in handling range changes.
…r label extraction - Changed the selector for extracting chart labels in the analytics dashboard spec from a generic attribute to specific IDs for the 24-hour and 7-day charts. - This improves the accuracy of the tests by ensuring they target the correct elements in the HTML response.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements to the analytics dashboard, focusing on enhanced user experience, better client-side behavior, and code maintainability. The most significant changes include switching the range selector to use a Stimulus controller, improving chart teardown to prevent stale chart rendering, disabling browser caching for analytics pages, and updating tests to reflect these changes.
Frontend improvements:
onchangehandler on the time range selector with a newanalytics-rangeStimulus controller (analytics_range_controller.js) for cleaner and more maintainable code. The controller updates the dashboard when the range changes. ([[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-a288c651d4e55a364877278cff95bc2f072a99934aeb03e515ce6859f5a0e5fbR1-R10),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-8f62f8b8d3dfa49981cbe3fe5bf80154bc3ad7b34b48b288f6ea52ae230376aeR8),[[3]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-8f62f8b8d3dfa49981cbe3fe5bf80154bc3ad7b34b48b288f6ea52ae230376aeR28),[[4]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-ce67901d4836649a597dfe441c2dcdd589ca29e7f42e74c4bd45f13e2fb9d198R9-R29))analytics-chartStimulus controller to properly tear down Chart.js instances before Turbo snapshotting and on disconnect, preventing stale or duplicate chart rendering when navigating back and forth. ([[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-6067580d43eabafa53531cc393f29df40011920fb5fe4e7545eb7b8849e912d9L14-R33),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-6067580d43eabafa53531cc393f29df40011920fb5fe4e7545eb7b8849e912d9L40-R60))data-turbo-temporaryand unique IDs to chart canvases, ensuring correct chart updates per selected range. ([[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-ce67901d4836649a597dfe441c2dcdd589ca29e7f42e74c4bd45f13e2fb9d198L63-R72),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-ce67901d4836649a597dfe441c2dcdd589ca29e7f42e74c4bd45f13e2fb9d198L82-R93))Backend and caching:
[[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-3f432ac853fbb152ccff55e6784ba665987262c3daf7f69094bf861c37daefbeR3),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-3f432ac853fbb152ccff55e6784ba665987262c3daf7f69094bf861c37daefbeR72-R77))allbefore grouping, improving clarity and future compatibility. ([[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-12bfe5b69e00cadf2c42e245972740a75fd72adc4c0d56fcee74b78e0b541860L21-R21),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-15d9e7595c3c05e91ea107a6340ce54b3a60f204f7de379f47f403930f469723L21-R21))Testing and validation:
[spec/requests/analytics_dashboard_spec.rbR40-R80](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-8466a0a2682559f7e2fb03285d72e0d82c755ece72f40ad5b34ad810277bd4dfR40-R80))Other minor changes:
nokogirigem version for security/compatibility. ([GemfileL50-R50](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-d09ea66f8227784ff4393d88a19836f321c915ae10031d16c93d67e6283ab55fL50-R50))[[1]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-6fc7229248a5c16634de6148a24d16bc5fb9e5eb90ea20aec54db788ecda7f50L12-L18),[[2]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-367c756924262acfe299ec48fe4086525c0a8ea68ba6f865a4f28e6e4fc32bfaL12-L18),[[3]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-1dd5a8f580b9615769ddc19a64a98f455d9d20eb149c443f41d9d3c797d39b12L12-L18),[[4]](https://github.com/lsa-mis/mi_classrooms/pull/444/files#diff-e02d060e76a92603bab92088890870d4c859fb237e9ccc4283766ca579acdddbL46-R46))