Geolocation Historical Data Records

Hello,

Yes, the suggested features are welcome to Smart IP module. The features you are suggesting are:

1. Geolocation historical data records with statistics table
2. Views integration of the historical and recent geolocation data
3. Search feature within the radius from a given zip for X month

Those features sounds good. Thanks.

Thank you for the update. Reminder: please make sure that the additional features are modular, so that the user have option to enable/disable them. Thanks!

Thanks lance.gliser. You're making a good progress.

That's great! Please give me time, I'm catching a deadline. Thank you.

I made a quick review of your patch. Here are my few comments:
- Why do we need to remove the variable_set('smart_ip_extract_zip_done', FALSE); in smart_ip.install?

  * Implements hook_schema().
  */
 function smart_ip_schema() {
   $schema['smart_ip'] = smart_ip_schema_definition_array();
+  $schema['smart_ip_detailed_statistics'] = smart_ip_detailed_statistics_schema_array();
+  $schema['smart_ip_summary_statistics'] = smart_ip_summary_statistics_schema_array();
   $schema['cache_smart_ip'] = drupal_get_schema_unprocessed('system', 'cache');
   $schema['cache_smart_ip']['description'] = 'Cache table for storing temporary data during Smart IP database update.';
   return $schema;
@@ -138,7 +361,6 @@
   variable_set('smart_ip_use_ipinfodb_service', FALSE);
   variable_set('smart_ip_correct_ipinfodb_key', FALSE);
   variable_set('smart_ip_get_zip_done', FALSE);
-  variable_set('smart_ip_extract_zip_done', FALSE);
   variable_set('smart_ip_store_location_csv_done', FALSE);
   variable_set('smart_ip_db_update_busy', FALSE);
   variable_set('smart_ip_location_csv_pointer', 0);
@@ -146,6 +368,9 @@
   variable_set('smart_ip_blocks_csv_pointer', 0);
   variable_set('smart_ip_blocks_csv_last_pointer', 0);
   variable_set('smart_ip_roles_to_geolocate', array(DRUPAL_AUTHENTICATED_RID));
+  variable_set('smart_ip_enable_statistics', 0);
+  variable_set('smart_ip_hours_between_fresh_geolocation', 24);
+  variable_set('smart_ip_days_to_keep_detailed_statistics', 365);
 }

- Please avoid escaping single quotes:

Translated strings where one can avoid escaping single quotes by enclosing the string in double quotes. One such string would be "He's a good person." It would be 'He\'s a good person.' with single quotes. Such escaping may not be handled properly by .pot file generators for text translation, and it's also somewhat awkward to read.

Can I ask for the source code? I'm getting error using my patch application.

I have used your demo application, its cool! However, it seems it captures the history of one user only, the admin (maybe because it's still in development stage). I noticed that this will be implemented as sub module, that's great! we can easily ported this feature to Drupal 7. Looking forward for the completion of this application. Thanks.

I'm sorry to confuse you. Yes, I understand that statistics is directly integrated into the main module, what I mean is that I saw a folder under modules directory of Smart IP with template files for creating new module which I assume that the statistics application will be created as sub module of Smart IP (which is the right move).

In my point of view, when our users start to test the statistics application they will expect immediately logs from their dummy visitors and some will think its not working with other users. If you designed that way because you think forcing a fresh geocode at login will affect the site performance, in my opinion its better to leave it that way or take no action at login. It will be unnoticeable in the long run.

I suggest adding in the documentation the simple installation and usage steps also. After this statistics application is completed future task for this:
- Port to Drupal 7
- Add IP lookup capabilities to the watchdog module (as in Smart IP issue: http://drupal.org/node/1047824). I think it is more appropriate to integrate this to your statistics application, what do you think?

If you’re available, why not? =) This is an easy task for the one who author the application and I can help in code review/testing… However, I can also do the porting but I will need your assistance because this application is huge compared to your last contribution.

Cool! I have tested the latest patch, it now display the latest log without waiting for the timestamp to expire. Now, I am currently reviewing the code...
I have question, why do we need to swap:

function smart_ip_set_location_data($account, $location) {
  global $user;
  if($account->uid == $user->uid){
    smart_ip_set_session_data($location);
  }
  smart_ip_set_user_data($account, $location);
}

to:

function smart_ip_set_location_data($account, $location) {
  global $user;
  if($account->uid == $user->uid){
    smart_ip_set_user_data($account, $location);
  }
  smart_ip_set_session_data($location);
}

I see, I think that would be very rare or may not be possible to get other user's location stored to another user's data.

I noticed that you simplified the:

function smart_ip_set_session_data($location) {
  // Store the location information in the session, merging any old data not overridden
  if (isset($_SESSION['smart_ip']['location'])) {
    $_SESSION['smart_ip']['location'] = array_merge($location, (array)$_SESSION['smart_ip']['location']);
  }
  else {
    $_SESSION['smart_ip']['location'] = $location;
  }
}

to:

function smart_ip_set_session_data($location) {
  // Store the location information in the session, merging any old data not overridden
  $_SESSION['smart_ip']['location'] = array_merge($location, (array)$_SESSION['smart_ip']['location']);
}

Using the simplified version would introduce bug when using the debug mode. Do we have concern if we still use the former?

At http://localhost/admin/reports/smart_ip_statistics/detailed
the filter field labeled with "And:" is confusing to the user... How does that field work?

I made some modification on statistics applcation. These modifications are just corrections, to comply with Drupal coding convetion. Like spaces in control statements, placement of commas. Please review the diff file I created.

* Just a reminder of the documentation.

Actually the attachment at #23 is already the latest version. I’ll apply the suggested label for your statistics views.

lance.gliser, I've sent you message in your contact form, did you received it?

Is it ok if you share your email?

Ok thanks. Already sent you a message.

The attachment is the latest version of Smart IP with statistics application patched. Please have a quick review then commit it to CVS.If you don’t have CVS account please apply now. You may want to use this thread for your CVS application.

Ok. Are you using the source code at comment #32 for your cleanup? For me that source code is ready for your CVS commit and if there are corrections found then it is strongly recommended be applied to that source code.

I hope you're fine now. I checked your CVS application and its status is "postponed (maintainer needs more info)". My suggestion is to follow up and mark your application as active (It seems you've already stated the needed info).

Subscribing.

has this been incorporated into the module?

Not yet, lance.gliser is responsible for this task.

lance.gliser, I see maybe a follow up can get their attention to review your pending request. Same here. Thanks =)

is this in d7?

No, this is in D6 only. For D7, you may want consider IP Geolocation Views and maps.

Drupal 6 is no longer supported