[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/geoip-ip-address-handling into lp:launchpad

2019-08-21 Thread noreply
The proposal to merge lp:~cjwatson/launchpad/geoip-ip-address-handling into 
lp:launchpad has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/geoip-ip-address-handling/+merge/369729
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/geoip-ip-address-handling into lp:launchpad

2019-08-18 Thread William Grant
Review: Approve code


-- 
https://code.launchpad.net/~cjwatson/launchpad/geoip-ip-address-handling/+merge/369729
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/geoip-ip-address-handling into lp:launchpad

2019-07-04 Thread Colin Watson
Colin Watson has proposed merging 
lp:~cjwatson/launchpad/geoip-ip-address-handling into lp:launchpad.

Commit message:
Revamp GeoIP IP address handling.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/geoip-ip-address-handling/+merge/369729

This removes the weird special case mapping private addresses to South Africa, 
and switches to the ipaddress module rather than ad-hoc private IP address 
detection, theoretically allowing us to handle IPv6 addresses too (although 
real IPv6 support requires switching to GeoIP2 data).

The ipaddress module was already in our virtualenv as an indirect dependency.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/geoip-ip-address-handling into lp:launchpad.
=== modified file 'lib/lp/answers/browser/tests/views.txt'
--- lib/lp/answers/browser/tests/views.txt	2018-12-07 13:24:51 +
+++ lib/lp/answers/browser/tests/views.txt	2019-07-04 18:55:19 +
@@ -573,14 +573,14 @@
 will contain languages from the HTTP request, or the most likely
 interesting languages based on GeoIP information.
 
-For example, if the user doesn't log in and their browser is configured to
-accept Brazilian Portuguese, the vocabulary will contain the languages
-spoken in South Africa (because the 127.0.0.1 IP address is mapped to
-South Africa in the tests).
+For example, if the user doesn't log in, their browser is configured to
+accept Brazilian Portuguese, and their request appears to come from a South
+African IP address, the vocabulary will contain the languages spoken in
+South Africa.
 
 >>> login(ANONYMOUS)
 >>> request = LaunchpadTestRequest(
-... HTTP_ACCEPT_LANGUAGE='pt_BR')
+... HTTP_ACCEPT_LANGUAGE='pt_BR', REMOTE_ADDR='196.36.161.227')
 >>> from lp.answers.browser.question import (
 ... QuestionLanguageVocabularyFactory)
 >>> view = getMultiAdapter((firefox, request), name='+addticket')
@@ -676,7 +676,7 @@
 database.
 
 >>> request = LaunchpadTestRequest(
-... HTTP_ACCEPT_LANGUAGE='fr, en_CA')
+... HTTP_ACCEPT_LANGUAGE='fr, en_CA', REMOTE_ADDR='196.36.161.227')
 
 >>> login(ANONYMOUS)
 >>> view = UserSupportLanguagesView(None, request)

=== modified file 'lib/lp/answers/stories/question-search-multiple-languages.txt'
--- lib/lp/answers/stories/question-search-multiple-languages.txt	2019-05-22 14:57:45 +
+++ lib/lp/answers/stories/question-search-multiple-languages.txt	2019-07-04 18:55:19 +
@@ -3,6 +3,12 @@
 By default, only questions written in English or one of the user
 preferred languages are listed and searched.
 
+Make the test browsers look like they're coming from an arbitrary South
+African IP address, since we'll use that later.
+
+>>> anon_browser.addHeader('X_FORWARDED_FOR', '196.36.161.227')
+>>> user_browser.addHeader('X_FORWARDED_FOR', '196.36.161.227')
+
 
 == Anonymous searching ==
 
@@ -29,10 +35,9 @@
 ... print(question.find('a').renderContents())
 Installation failed
 
-The questions match the languages inferred from by GeoIP
-127.0.0.1 is mapped to South Africa in the test suite. The language
-control shows the intersection of the user's languages and the
-languages of the target's questions. In this example:
+The questions match the languages inferred from GeoIP (South Africa in
+this case). The language control shows the intersection of the user's
+languages and the languages of the target's questions. In this example:
 set(['af', 'en', 'st', 'xh', 'zu']) & set(['en', 'es']) == set(en).
 
 >>> sorted(anon_browser.getControl(name='field.language').options)
@@ -113,6 +118,10 @@
 anonymous user makes a request from a GeoIP that has no languages
 mapped, we assume they speak the default language of English.
 
+>>> for pos, (key, _) in enumerate(anon_browser.mech_browser.addheaders):
+... if key == 'X_FORWARDED_FOR':
+... del anon_browser.mech_browser.addheaders[pos]
+... break
 >>> anon_browser.addHeader('X_FORWARDED_FOR', '172.16.1.1')
 >>> anon_browser.open(
 ... 'http://launchpad.test/ubuntu/+source/mozilla-firefox/+questions')
@@ -166,6 +175,10 @@
 When the project languages are just English, and the user speaks
 that language, we do not show the language controls.
 
+>>> for pos, (key, _) in enumerate(user_browser.mech_browser.addheaders):
+... if key == 'X_FORWARDED_FOR':
+... del user_browser.mech_browser.addheaders[pos]
+... break
 >>> user_browser.addHeader('X_FORWARDED_FOR', '172.16.1.1')
 >>> user_browser.open(
 ... 'http://launchpad.test/ubuntu/+source/mozilla-firefox/+questions')

=== modified file 'lib/lp/services/geoip/configure.zcml'
--- lib/lp/services/geoip/configure.zcml	2010-10-25 05:33:20 +
+++ lib/lp/services/geoip/configure.zcml	2019-07-04 18:55:19 +
@@ -1,4 +1,4 @@
-
 
@@ -34,9 +34,4