I keep finding myself in situations where due to caching requirements it's more practical to use JavaScript for country specific functionality than doing it server side. I don't have a personal need for the city level lookup so I'm not spec-ing that out at this point but I've tried to choose names that don't preclude adding it later.

Module requirements:

  • Add a menu callback geoip/json/country that looks up the visitor's information and uses drupal_json() to return the value.
  • When in debug mode we need to continue to allow the IP to be specified.
  • We need to disable page caching for these requests: $GLOBALS['conf']['cache'] = CACHE_DISABLED;
  • Add a helper function to handle calling drupal_add_js() for our JS library.

JS requirements:

  • Create a function named Drupal.geoip.lookupCountry to make the AJAX requests to the server (Drupal.settings.basePath + 'geoip/json/country').
  • The function should use a cookie to avoid repeated requests.
  • The cookie lifetime should be overridable (perhaps by checking Drupal.settings.geoip.lifetime for a value?) but default to 24 hours.
  • The cookie path should be set to Drupal.settings.basePath.
  • I'd prefer to avoid adding an external dependency on a cookie library, it should be simple enough to load and save the country code.

Comments

bobooon’s picture

Status: Active » Needs review
StatusFileSize
new1.2 KB
new1.54 KB

Attached is a patch and new JS file that adds country lookup support to GeoIP API.

drewish’s picture

I haven't opened up the JS since it's in a zip file but curious about the anti-caching code in hook_init(). For some reason I was thinking we could do that in the page callback. Does it have to be in hook_init?

bobooon’s picture

I'm actually not sure if it needs to be in hook_init() or in the page callback. It just seemed like hook_init() would be the safer choice, what do you think?

bobooon’s picture

Also, should we use jquery.cookie to handle cookie read/writes?

bobooon’s picture

StatusFileSize
new1.46 KB

New JS file corrects debug mode when using the JSON callback.

drewish’s picture

Cool the text file is much easier to read. We should replace all the tabs with spaces in there and make sure we're using {} to wrap blocks of code in if else statements.

Looking at the lookupCountry() now I'm wondering if it makes more sense to have the caller pass in a callback function that we call from the AJAX call's success function. That way we don't have to block while we're waiting on the request. What do you think?

I kind of feel like the debug bit is a little weird. What do you think about trying to pull the geoip_debug from window.location.search ?

bobooon’s picture

StatusFileSize
new1.49 KB
new1.43 KB

Attached is a new patch file and javascript file. I updated the lookupCountry function to call a passed in function. I also added in the wondow.location.search regular expression for debug mode. Below is an example of how you would call lookupCountry and define a callback function.

Drupal.geoip.lookupCountry('Drupal.geoip_test');

bobooon’s picture

StatusFileSize
new1.53 KB

Slightly updated javascript file that allows an extra parameter to be passed to the callback.

drewish’s picture

StatusFileSize
new3.49 KB

here's a re-roll of another patch that rob had sent me.

drewish’s picture

StatusFileSize
new4.19 KB

my re-roll dropped two functions, the json callback and the helper to add the js (which i've renamed geoip_add_js()). i also wrapped all the JS in a closure.

drewish’s picture

StatusFileSize
new4.15 KB

Tightened up the regex a bit. Also removed the ability to pass the ip back as an argument. It seemed unused and made things a bit confusing.

drewish’s picture

StatusFileSize
new2.92 KB

Noticed a few issues with that last patch. Looks like if you're on a private IP e.g. 192.168.1.1 and the lookup fails the 'success' callback never fires. I feel like it should either fire or we should accept an error callback. For simplicity I just casted the result to a string so *something* comes back. Also fixed an undefined variable warning and cleaned up a couple of other small formatting issues.

drewish’s picture

StatusFileSize
new2.97 KB

Fixed some bad comments.

drewish’s picture

Status: Needs review » Fixed

Committed to 6.x-1.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.