Good morning,

I'm looking at using this module for an upcoming site, and have a list of things I believe would be beneficial to all that we need. I'd like to see what you want developed in a patch for your module, and what should be kept in a site specific module for me.

Here's my expected list of additions:

  • Load goelocation data for all visitors, without login.
  • If the location module is installed and user locations are allowed:
    • If the user not logged in, copy applicable geolocation data into the user's first location slot. This would be to allow more modules to rely on the location data, and you would provide it.
    • If location information is collected in the registration screen, pre-fill from geolocation data. Merge any remaining data from geolocation into the user's first location if the rest of the information matches when the account is created.
    • If location information is not collected, fill the user's first location from just geolocation data.

All of this could be a checkbox option in the settings if included into this module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

arpeggio’s picture

@lance.gliser thank you for the nice suggestions. Of course, your patch for these features are welcome. I am thinking it would be better if we make your suggested features as sub module for smart_ip just like the device_geolocation. I would suggest to call it as "Smart IP/Location API bridge". Smart IP will provide hooks for this sub module if needed.

lance.gliser’s picture

@arpeggio I've been working on the interplay of these two modules for a bit since I posted this. I've got some ideas I'll play with, and hand over as a sub module when they are finished.

I would argue one point though. I think it would make more sense to allow the first point, geolocation without login, from the main module through an option. Functionality being the same, and the only difference as time.

Select box:
GeoLocation users:
[] Immediately
[] During Login and Registration (Default)

arpeggio’s picture

@lance.gliser no problem, we can add option for mode of saving data at the Smart IP admin page. Please help provide patch for this feature. Thanks.

lance.gliser’s picture

Will do.

And you're interested in the development of integration between smartIP and location_user, watch http://drupal.org/node/991578.

lance.gliser’s picture

Title: Some Improvements. Would you like them? » Create option to fire geotargeting for anonymous users
Version: 6.x-1.0-alpha1 » 6.x-1.1-alpha1
Component: Miscellaneous » Code
Assigned: Unassigned » lance.gliser
lance.gliser’s picture

Title: Create option to fire geotargeting for anonymous users » Create option to fire geotargeting by role

After a bit of designing this, I have decided to change out the expected option. There's more potential for control if we go by role than just logged in for not.

lance.gliser’s picture

Status: Active » Needs review
FileSize
9.84 KB
9.86 KB

I'm attaching a patch that alters the .module, .install, and admin.inc files as well as my files zippped. I've externalized a couple functions in case other utilities should need them later while developing. It works on my local box, but I would suggest reading through the changes in depth to ensure things are done the way you like.

arpeggio’s picture

Status: Needs review » Closed (fixed)

Nice work lance.gliser, I have already reviewed and tested the patch you have submitted for this request feature. However, there's a minor bug with admin spoofing when the patch applied but I already fixed it. The option "Save user location upon login" becomes redundant and is now replaced with geotargeting by role (which is also enabling the authenticated user option), so that option is now removed. Thanks.

arpeggio’s picture

BTW, all Smart IP D6 commits are now in Drupal-6--1 branch. Before I was committing the Smart IP D6 modifications at the wrong branch (HEAD). And also this feature geotargeting by role is now ported to Smart IP D7. Thanks.

lance.gliser’s picture

Happy to help. And I think that's the last of the things I've found to do for this module specifically. Might it be time to put out a new version release? New features, critical bug fix, minor bug fix. I'd love to have a non-patched version in my system, unless you're planning something else?

arpeggio’s picture

Ah yes, I've put out a new feature/bug fix release for Smart IP D6 and D7. Thank you for reminding.

arpeggio’s picture

@lance.gliser there's an issue regarding this feature please check http://drupal.org/node/1076638.

lance.gliser’s picture

Noted.