Updated: Comment #N
Problem/Motivation
The Statistics module has one job: to count hits to nodes; in D8 HEAD, this is simply not happening. The root cause seems to be that neither the statistics.js file nor the necessary drupalSettings items are being included on any node page. This could be because of a faulty hook_node_view() implementation, or the hook doesn't exist anymore. It's also possible that the way those scripts/settings are being attached is no longer valid.
Marking as critical since this is literally the only thing the Statistics module does and it's broken.
Proposed resolution
We need to get these assets back on node pages; we also need to fix the tests to ensure they're included.
Remaining tasks
Fix, patch, test, etc.
User interface changes
n/a
API changes
n/a
Comments
Comment #1
iamEAP CreditAttribution: iamEAP commentedComment #2
iamEAP CreditAttribution: iamEAP commentedComment #3
iamEAP CreditAttribution: iamEAP commentedNeeds tests.
Comment #4
iamEAP CreditAttribution: iamEAP commentedTests.
Comment #7
iamEAP CreditAttribution: iamEAP commented4: drupal-ensure_stats_js_included-2172859-4-tests_only.patch queued for re-testing.
Comment #8
iamEAP CreditAttribution: iamEAP commentedJust cleaning up test comments. Reviews welcome.
Comment #11
iamEAP CreditAttribution: iamEAP commentedComment #12
nod_That works for me, thanks :)
Comment #13
webchickWhy did we remove the tests that ensure Statistics module continue to log on cached pages? That's also part of the only thing that Statistics module does. :)
Comment #14
nod_Well stats are JS only so cached or not as long as there is the stat JS file and the proper settings it will be logged.Oh I see what you mean. Not sure why :p because the actual logging doesn't depend on cache enabled or not? The logging always goes through statistics.php and that's not a page that drupal can even cache.
Comment #15
iamEAP CreditAttribution: iamEAP commentedYup, nod_ covered it. The cached vs. not cached check was important when logging was done in a hook_exit(), but now that it's an asynchronous script + an uncacheable callback, it's not a concern.
Would seem like a waste of test cycles to make sure a piece of JS is still there after a page is cached.
Comment #16
catchYeah if we had real browser testing we could do a meaningful test here, but we don't so it would only be checking the js file is added on the cached page. Moving back to RTBC but I didn't review the actual patch here so not committing yet.
Comment #17
webchickOk, thanks that makes sense.
Committed and pushed to 8.x. Thanks!