Posted by xjm on November 29, 2012 at 2:18am
10 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | statistics.module |
| Category: | task |
| Priority: | normal |
| Assigned: | damiankloip |
| Status: | closed (fixed) |
| Issue tags: | VDC |
Issue Summary
Part of meta #1853522: [META] (Re)introduce Views data integration for core modules. From the 8.x-3.x branch of Views.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| statistics.patch | 8.02 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 48,908 pass(es). | View details |
Comments
#1
The last submitted patch, statistics.patch, failed testing.
#2
#3
I'm working on this one. Hopefully this will give us a better idea/template to base the other integration tests on.
#4
Ok, here is a more generic test to just test the integration of the fields, so we have some coverage. I think we agree that creating views to cater for all filter/arg combinations isn't feasible. Also added a test for the AccessLogPath handler.
#5
Without the annoying no newline..
#6
Just some general cleanup
#7
Ups totally wrong interdiff.
#8
The last submitted patch, drupal-1853540-6.patch, failed testing.
#9
Yeah, those changes look like good ones to me. Just need to fix up that init() method I think...
#10
Oh right, so we have a proper test coverage, let's get it back.
#11
The patch itself looks good to me.
Do we want to update the statistics module to take advantage of this (dog-fooding) or is that not really an option?
Seems like a recipe for performance issues though. These tables get really big.
#12
That sounds like a "needs review."
Also note the existence of #1446956: Remove the accesslog from statistics. It's possible we want to postpone this issue until something's decided over there.
#13
I would personally vote to do the integration first, because we have another issue which describer which parts of drupal we want to replace: #1864980: [meta] Figure out how to integrate Views into core
#14
Hm. but if the accesslog table goes away entirely, including all of the faculties of tracking hostname, locations, hits, etc... then we don't need any Views integration nor any default views for that functionality. Or do I misunderstand?
#15
@webchick
Absolute! Maybe we should simply just integrate the node_counter table for now?
#16
Yeah, that would be safer for now, I think. Would probably also help with Dries's performance/big table concern. We could file a postponed follow-up for the accesslog table, pending resolution of that issue.
#17
Let's rerole after removing of accesslog.
#18
Simple is good, Let's get it in!
#19
Can we get the followup for the accesslog linked here, presumably postponed on #1446956: Remove the accesslog from statistics?
Edit: Never mind, no followup needed. Dries committed that in #1446956-40: Remove the accesslog from statistics; the issue is just NR for a changelog entry. ;)
#20
Is there a follow-up issue that deletes the old/existing statistics module table in favor of a view? I can't seem to find it.
#21
I'm confused about your last comment dries.
Didn't we killed the feature of the old/existing statistics module tables with the accesslog patch from above? #1446956: Remove the accesslog from statistics
Just checked and statistics module is just about count node views, but sure we could build a view for that.
#22
Yes the listing is gone, this is just integration for the count statistics for which there's no list display in core at all.
Committed/pushed to 8.x.
#23
Automatically closed -- issue fixed for 2 weeks with no activity.