Needs work
Project:
Inactive User
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
26 Jan 2012 at 00:01 UTC
Updated:
31 May 2012 at 22:18 UTC
Jump to comment: Most recent file
I'm trying to convert inactive_user to Drupal 7 and could use some help with hook_cron(). My work so far includes:
inactive_user_custom_settings() seems fine as does inactive_user_schema(). I'm asking for help to make the queries in inactive_user_cron() actually do something.
Since db_fetch_object() and db_affected_rows() were deprecated in D7, the structure of the queries needs to be changed. I think I'm on the right track but am missing some knowledge of DBTNG.
I've attached a patch of my work. Thanks for your help.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | inactive_user-d7_db_and_test_fixes-1417114-2.patch | 32.87 KB | thebruce |
| inactive_user_d7.patch | 29.54 KB | esod |
Comments
Comment #1
esod commentedI guess the lack of response to this issue is because my patch had all kinds of context switching. I'll rephrase my question and describe where I'm stuck.
In Drupal 6, a query in inactive_user_cron is:
For Drupal 7, I've written the same query as:
Looking at the $users query, for instance, and knowing that db_fetch_object() has been deprecated in Drupal 7, I thought up:
instead of:
For Drupal 6.
If someone can help me with this one query, I'm pretty sure I can apply that knowledge to the rest of queries in inactive_user_cron, at which point the module will be upgraded to Drupal 7.
Thanks for your help.
Comment #2
thebruce commentedHi,
Here is a patch with updates to the module and tests. The module works to the tests and works functionally.
I deviated a bit from the plan above in the query structure, on the selects I used db_select. I realize this has some performance concerns as indicated by Crell in the select api docs comments. What I thought this might lead to is the the creation of an implementing class where, since many of the criteria clauses are re-used, could be made methods of the inactive user db object. I think this might be a fair choice for extension and maintenance.
Anyway, I wanted to get the patch for the straight up port contributed since this issue had been outstanding for quite a while.
Hope this helps.
----
thebruce
Comment #3
rgristroph commentedI looked over this patch, and tried it out.
It looks good to me. The only comments I have are:
I don't think any of those things should stop this patch from going forward, it's a big improvement. Doing all the queries correctly is a big win.
--Rob
Comment #4
deekayen commentedI committed #2 just to keep things moving.
The guide you referenced is probably https://drupal.org/empty-git-master.
Comment #5
rgristroph commentedThanks !
Just to confirm, since I did the test of the patch at #2 my test site has sent the appropriate notifications on the appropriate users I created, so it does seem to work right.
Besides the two things I noted - a link to the configuration and numeric vs. human readable times - is there anything else keeping this issue from being actually closed ?
--Rob