I just ran a few queries against the d.o usage data:
SELECT count(t1.pid) FROM project_usage_day t1 INNER JOIN project_usage_day t2 ON t1.timestamp = t2.timestamp AND t1.site_key = t2.site_key AND t1.tid = t2.tid WHERE t1.pid = 29351 AND t2.pid = 38878 AND t1.tid = 87 AND t1.timestamp = 1253232000;
t1.pid == 29351 is signup
t2.pid == 38878 is views
t1.tid == D6 version
So, this query tells me how many sites reported to d.o that were using both signup and views:
+---------------+
| count(t1.pid) |
+---------------+
| 2272 |
+---------------+
1 row in set (1.39 sec)
In the same week, how many were using D6 signup at all:
SELECT count(pid) FROM project_usage_day WHERE pid = 29351 AND tid = 87 AND timestamp = 1253232000;
+------------+
| count(pid) |
+------------+
| 2481 |
+------------+
1 row in set (0.97 sec)
So, we've got 2272 / 2481 signup sites that use views, or 91.6%.
There's a *lot* of code in signup to provide (inflexible) fallbacks for various listings when views isn't enabled. It'd certainly make maintaining the code easier, and probably using the module more simple, if views was just a required dependency.
Thoughts?
-Derek
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 582986-2.signup_require_views.patch | 35.34 KB | dww |
Comments
Comment #1
dwwFYI: I started a poll about this on g.d.o in the "Event management systems" group: Should the signup module require the views module?...
Comment #2
dwwOn a whim, I wrote a patch to rip out all the non-views fallback code. Even with a DB update to fix a few variables, and edits to the documentation, the patch removes 491 lines and only adds 131. ;) This doesn't even count what we might win by converting admin/content/signup to a view over at #583196: Fix the admin/content/signup administration overview page to better handle signup limits and not suck so much ...
This is awfully tempting...
Comment #3
dwwOh, I should mention: unless you have VBO installed, this patch would entail a loss of some functionality at node/N/signups/admin. You need VBO if you want to be able to bulk cancel signups, etc. Of the 2481 sites using signup in the time period I'm querying, only 231 of them have VBO installed, so only about 9%. :( So, if we go this route, it might be worth a hook_requirements() check or something warning site admins to install VBO or check a checkbox to acknowledge they don't care or something...
Comment #4
dwwHere's a perfect example of why the status quo sucks, and why I'd rather not spend any more time on the non-views case:
#589030: No 'Edit signup' link on the node/N/signups/admin tab
Comment #5
dwwHere's another example of a related issue: #547758: exclude unpublished nodes from admin signup list
Comment #6
dwwAnd another: #376172: Add a filter in the signup overview
Comment #7
ezra-g commentedWe've discussed this in IRC a while back and I'm in favor of this change for the duplication it eliminates and the benefits users get through using Views. As dww points out, user interest in those benefits, like adding filters to administrative listings, is clearly there. Also the poll dww created shows 91% (32 votes) in favor of eliminating non-Views code from Signup. This is interesting to me because it closely matches the percent of Signup-enabled sites that are already running Views.
Particularly before we spend time updating code for D7, I say we remove this code.
In my testing of this patch I found that the administrative signup listing didn't show up because of a
variable_get('signup_display_signup_admin_list', 'signup')that needed its default value changed to 'embed-view'. With this change, and another re-roll for a hunk that didn't apply, I've committed.Goodbye to some extra cruft code :).
http://drupal.org/cvs?commit=470392
Comment #8
ezra-g commentedand no_views.inc removed with http://drupal.org/cvs?commit=470394.
Comment #9
ezra-g commentedThis fixed #629220: Anonymous users not shown in admin interface as well :).
Comment #10
dwwWow, I just looked at my issue queues again and saw this amazing flurry of signup activity. ;) Thanks ezra-g!
Shouldn't this (and all the other recent fixes to 6.x-1.x-dev) actually be marked as 6.x-2.x-dev to-be-ported so that joachim can get his branch in sync?
Thanks again, this is amazing progress!
Cheers,
-Derek
Comment #11
magnus commentedGreat work Ezra, nice to see the issue queue got cleaned up!
Comment #12
dwwWhy does includes/no_views.inc still exist?
/me wonders why he's so closely reviewing everything *after* creating 6.x-1.0. ;)
Comment #13
ezra-g commentedAs mentioned #8, I thought I removed that with http://drupal.org/cvs?commit=470394 but apparently it's still there.
Since we've removed the Views dependency already, let's address this in #1020590: remove no_views.inc to avoid confusion about the status here.