search

in2code-de / ipandlanguageredirect

TYPO3 redirect - browserlanguage and ip-address based
GNU General Public License v3.0
14 stars 14 forks source link

[TASK] set ipAddress also when 'null' (string) and make sure '?key=' is added to IpApi key #6

Closed pixelmatseriks closed 6 years ago

pixelmatseriks commented 6 years ago

[TASK] also set ipAddress when it is 'null' (string) [TASK] make sure '?key=' is added to IpApi key

einpraegsam commented 6 years ago

Can you please explain what kind of issue your both changes are solving? 1) I don't understand the comparization to the string "null" 2) It seems that this is a bugfix. Please explain how to reproduce this bug.

pixelmatseriks commented 6 years ago

  1. I was checking why no ipAddress was fetched, and debugged condition $ipAddress === null. The condition wasn't true even though output of $ipAddress was null. Output of gettype($ipAddress) showed string instead of NULL. The condition $ipAddress == 'null' matches the null as string. (TYPO3 8, PHP 7.0.22-0ubuntu0.16.04.1)

  2. When ipApiKey is added in extension manager settings, the url to ipapi should be: https://ipapi.co/ipaddress/json/?key=someapikeyfromextsettings instead of just https://ipapi.co/ipaddress/json/. It works without this commit if the part ?key= is included in extension manager settings, but with this change it will work also if the ?key= part isn't included.

einpraegsam commented 6 years ago
  1. It seems that you're right. But the problem is located in JavaScript and not in PHP. At the moment a default AJAX request is https://domain.org/?type=1555&tx_ipandlanguageredirect_pi1[browserLanguage]=de&tx_ipandlanguageredirect_pi1[ipAddress]=null&tx_ipandlanguageredirect_pi1[referrer]=&tx_ipandlanguageredirect_pi1[languageUid]=0&tx_ipandlanguageredirect_pi1[rootpageUid]=1&tx_ipandlanguageredirect_pi1[countryCode]= I will fix that in JavaScript.

2) Ok seems that this is an improvement - I'm going to accept this. Nevertheless can you please change your PR to only change the $key? After that I will merge it.

Thx for the time that you spent into that issue.

einpraegsam commented 6 years ago

Thx, I will close this one. See other PR https://github.com/einpraegsam/ipandlanguageredirect/pull/7 BTW: I released a new version what fixes the "null" issue yesterday

pixelmatseriks commented 6 years ago

Great thanks :) :+1: (made a new PR instead of changing this, I still use the fork temporary in a project.)