When entering the API key for the IPInfoDB service for the first time this message pops up.

Error message
Notice: Undefined variable: ip in smart_ip_admin_settings_submit() (line 350 of sites/all/modules/smart_ip/includes/smart_ip.admin.inc).

This is using drupal 7.22

CommentFileSizeAuthor
#8 IPInfo_API_key_enter-1983568_1.patch1.04 KBlostkangaroo
#6 IPInfo_API_key_enter-1983568.patch1.35 KBlostkangaroo
#3 IPInfo_API_key_enter-1983568-3.patch1.01 KBlostkangaroo
#1 smartip_undefined_variable_350.jpg68.01 KBmveplus

Comments

mveplus’s picture

mveplus commented
StatusFileSize
new smartip_undefined_variable_350.jpg68.01 KB

Just installed Drupal 7.22 and Smart IP, can confirm this issue as well...

Undefined variable

IP is not served, but Country, Region etc.. work.

kingfisher64’s picture

kingfisher64 commented
Priority: Minor » Major

Since the IP is not served I think it's more than a minor issue so bumping this to major.

lostkangaroo’s picture

lostkangaroo commented
Assigned: Unassigned » lostkangaroo
Priority: Major » Normal
Status: Active » Needs review
StatusFileSize
new IPInfo_API_key_enter-1983568-3.patch1.01 KB

It looks like the IP address used was not being used in the drupal_set_message calls causing the notices when the debug setting was turned off. This is what I noticed at least when I started investigating this issue. Try this patch and let me know if it solves your problem.

Also setting this to normal as the IP in the submit form is indeed being passed which can be verified by doing a dpm() on the returned $location array.

kingfisher64’s picture

kingfisher64 commented

This was tested against:

7.x-2.0 2013-Mar-26
7.x-1.9 2013-Feb-04 &
7.x-1.x-dev 2013-Mar-28 versions

patching file smart_ip.admin.inc
Hunk #1 FAILED at 347.
Hunk #2 FAILED at 384.
2 out of 2 hunks FAILED -- saving rejects to file smart_ip.admin.inc.rej
lostkangaroo’s picture

lostkangaroo commented

That's interesting I will look into this right now.

lostkangaroo’s picture

lostkangaroo commented
StatusFileSize
new IPInfo_API_key_enter-1983568.patch1.35 KB

Alright rerolled the patch and tested its ability to apply and came back clean.

kingfisher64’s picture

kingfisher64 commented

Hi lostkangaroo, sorry i'm not getting sucess when applying the patch. The following displays when applied against stable 2.x branch. I'm (stuck) using a windows environment so am using cygwin but normally don't have any problems. Just trying to think why it's coming back clean for you and saying it's already applied for me?

$ patch < IPInfo_API_key_enter-1983568.patch
patching file smart_ip.admin.inc
patching file smart_ip.module
Reversed (or previously applied) patch detected!  Assume -R? [n]

Many thanks :)

lostkangaroo’s picture

lostkangaroo commented
StatusFileSize
new IPInfo_API_key_enter-1983568_1.patch1.04 KB

Try this one. It removes the second bit that was harmless to me but seems to be messing with you.

kingfisher64’s picture

kingfisher64 commented

It's not having it again kangaroo, sorry.

$ patch < IPInfo_API_key_enter-1983568_1.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/includes/smart_ip.admin.inc b/includes/smart_ip.admin.inc
|index ec21a53..a48494a 100644
|--- a/includes/smart_ip.admin.inc
|+++ b/includes/smart_ip.admin.inc
lostkangaroo’s picture

lostkangaroo commented

Alright then we will have to do this the manual way. There are 2 lines you will need to change.
Line 350

Find:       '%ip'      => $ip,
Replace:    '%ip'      => $location['ip'],

Line 387

Find:       drupal_set_message(t('IP Address @ip is not assigned to any location.', array('@ip' => $ip)));
Replace:    drupal_set_message(t('IP Address @ip is not assigned to any location.', array('@ip' => ip_address())));

This is all the patches from above were going to do. Good luck and let me know if the error message becomes a thing of the past so we can mark this RTBC and watch it go green.

kingfisher64’s picture

kingfisher64 commented

Well using the IP lookup functionality now works so I'm assuming this is part and parcel of the issue. I'm seeing no error at present.

Thank you - for your time and patience :)

lostkangaroo’s picture

lostkangaroo commented
Status: Needs review » Reviewed & tested by the community

No worries on a fresh install I was having the same problems so hopefully soon this will be rolled into the current releases to prevent any future admins from having the same experiences.

arpeggio’s picture

arpeggio commented
Status: Reviewed & tested by the community » Fixed

Hi, thank you for reporting this issue kingfisher64 also for sharing the patch lostkangaroo. However, the fix for Line 387:

Find:       drupal_set_message(t('IP Address @ip is not assigned to any location.', array('@ip' => $ip)));
Replace:    drupal_set_message(t('IP Address @ip is not assigned to any location.', array('@ip' => ip_address())));

IP address is not always the IP of the current user (ip_address()), but it can also be the value supplied at "IP address to use for testing" field. Anyway, this issue is already fixed. Please use the dev version. Thanks.

kingfisher64’s picture

kingfisher64 commented

I'm only seeing a 2.x branch stable and the 1.x branch for dev. Are we to be using the 1.x branch? I'm assuming so, it's also no biggie to just copy & paste according to #13 for the 2.x branch.

Would just appreciate some clarification.

Thank you for your time and an excellent module arpeggio :)

arpeggio’s picture

arpeggio commented

I'm sorry, I forgot to mentioned that the fix is already pushed, please use the dev version (7.x-1.x-dev).

You're welcome kingfisher64.

Thanks.

lostkangaroo’s picture

lostkangaroo commented
Assigned: lostkangaroo » Unassigned

No worries arpeggio the code in #13 is simply there to help address any cases when $ip wasn't assigned as a fallback until you could take a look at it.

Status: Fixed » Closed (fixed)

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