launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/filebug-better-info into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/filebug-better-info/+merge/109948


-- 
https://code.launchpad.net/~jcsackett/launchpad/filebug-better-info/+merge/109948
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/filebug-better-info into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-06-08 20:58:13 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-06-12 22:17:21 +0000
@@ -278,6 +278,8 @@
             type.name for type in PRIVATE_INFORMATION_TYPES]
         cache.objects['show_information_type_in_ui'] = (
             self.show_information_type_in_ui)
+        cache.objects['show_userdata_as_private'] = bool(getFeatureFlag(
+            'disclosure.display_userdata_as_private.enabled'))
         cache.objects['bug_private_by_default'] = (
             IProduct.providedBy(self.context) and self.context.private_bugs)
         cache.objects['information_type_data'] = [

=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js	2012-06-07 12:17:48 +0000
+++ lib/lp/bugs/javascript/filebug.js	2012-06-12 22:17:21 +0000
@@ -51,12 +51,30 @@
     }
 };
 
+var get_new_banner_text = function(value) {
+    var info_type_descriptions = {
+        EMBARGOEDSECURITY: 'Embargoed Security',
+        USERDATA: 'User Data',
+        PROPRIETARY: 'Proprietary'
+    };
+
+    if (LP.cache.show_userdata_as_private) {
+        info_type_descriptions.USERDATA = 'Private'; 
+    }
+    var text_template = "This report has {info_type} information." + 
+        " You can change the information type later.";
+    value = info_type_descriptions[value];
+    return Y.Lang.substitute(text_template, {'info_type': value});
+}
+
 var setup_information_type = function() {
     var itypes_table = Y.one('.radio-button-widget');
     itypes_table.delegate('change', function() {
+        var banner_text = get_new_banner_text(this.get('value'));
         var private_type = (Y.Array.indexOf(
             LP.cache.private_types, this.get('value')) >= 0);
-        update_privacy_banner(private_type);
+        
+        update_privacy_banner(private_type, banner_text);
     }, "input[name='field.information_type']");
 };
 

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.js'
--- lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-01 05:11:24 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-12 22:17:21 +0000
@@ -85,8 +85,8 @@
             Y.Assert.isNull(banner_hidden);
             var banner_text = Y.one('.banner-text').get('text');
             Y.Assert.areEqual(
-                'This report will be private. ' +
-                'You can disclose it later.', banner_text);
+                'This report has Embargoed Security information. ' +
+                'You can change the information type later.', banner_text);
         },
 
         // Selecting a public info type using the legacy radio buttons
@@ -214,8 +214,8 @@
             Y.Assert.isNull(banner_hidden);
             var banner_text = Y.one('.banner-text').get('text');
             Y.Assert.areEqual(
-                'This report will be private. ' +
-                'You can disclose it later.', banner_text);
+                'This report has User Data information. ' +
+                'You can change the information type later.', banner_text);
         },
 
         // Selecting a public info type using the popup choice widget

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-06-12 16:25:42 +0000
+++ lib/lp/registry/model/person.py	2012-06-12 22:17:21 +0000
@@ -271,6 +271,7 @@
     IEmailAddress,
     IEmailAddressSet,
     InvalidEmailAddress,
+    VALID_EMAIL_STATUSES,
     )
 from lp.services.identity.model.account import Account
 from lp.services.identity.model.emailaddress import (
@@ -3375,13 +3376,14 @@
     def ensurePerson(self, email, displayname, rationale, comment=None,
                      registrant=None):
         """See `IPersonSet`."""
-        person = getUtility(IPersonSet).getByEmail(email)
-
+        person = getUtility(IPersonSet).getByEmail(
+                    email,
+                    filter_status=False)
+        
         if person is None:
             person, email_address = self.createPersonAndEmail(
                 email, rationale, comment=comment, displayname=displayname,
                 registrant=registrant, hide_email_addresses=True)
-
         return person
 
     def getByName(self, name, ignore_merged=True):
@@ -3597,28 +3599,32 @@
         except SQLObjectNotFound:
             return None
 
-    def getByEmail(self, email):
+    def getByEmail(self, email, filter_status=True):
         """See `IPersonSet`."""
-        address = self.getByEmails([email]).one()
+        address = self.getByEmails([email], filter_status=filter_status).one()
         if address:
             return address[1]
 
-    def getByEmails(self, emails, include_hidden=True):
+    def getByEmails(self, emails, include_hidden=True, filter_status=True):
         """See `IPersonSet`."""
         if not emails:
             return EmptyResultSet()
         addresses = [
             ensure_unicode(address.lower().strip())
             for address in emails]
-        extra_query = True
+        hidden_query = True
+        filter_query = True
         if not include_hidden:
-            extra_query = Person.hide_email_addresses == False
+            hidden_query = Person.hide_email_addresses == False
+        if filter_status:
+            filter_query = EmailAddress.status.is_in(VALID_EMAIL_STATUSES)
         return IStore(Person).using(
             Person,
             Join(EmailAddress, EmailAddress.personID == Person.id)
         ).find(
             (EmailAddress, Person),
-            EmailAddress.email.lower().is_in(addresses), extra_query)
+            EmailAddress.email.lower().is_in(addresses),
+            filter_query, hidden_query)
 
     def _merge_person_decoration(self, to_person, from_person, skip,
         decorator_table, person_pointer_column, additional_person_columns):

=== modified file 'lib/lp/registry/tests/test_person_vocabularies.py'
--- lib/lp/registry/tests/test_person_vocabularies.py	2012-05-31 08:44:50 +0000
+++ lib/lp/registry/tests/test_person_vocabularies.py	2012-06-12 22:17:21 +0000
@@ -21,6 +21,7 @@
     )
 from lp.registry.vocabularies import ValidPersonOrTeamVocabulary
 from lp.services.identity.interfaces.account import AccountStatus
+from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
 from lp.services.webapp.vocabulary import FilteredVocabularyBase
 from lp.testing import (
     login_person,
@@ -370,6 +371,14 @@
         # The vocab shouldn't support person or team filters.
         self.assertEqual([], self.getVocabulary(None).supportedFilters())
 
+    def test_unvalidated_emails_ignored(self):
+        person = self.factory.makePerson()
+        unvalidated_email = self.factory.makeEmail(
+            'fnord@xxxxxxxxxxx',
+            person,
+            email_status=EmailAddressStatus.NEW)
+        search = self.searchVocabulary(None, 'fnord@xxxxxxxxxxx')
+        self.assertEqual([], [s for s in search])
 
 class TestNewPillarShareeVocabulary(VocabularyTestBase,
                                         TestCaseWithFactory):

=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py	2012-05-28 10:46:28 +0000
+++ lib/lp/registry/tests/test_personset.py	2012-06-12 22:17:21 +0000
@@ -115,6 +115,15 @@
             "PersonSet.getByEmail() should ignore case and whitespace.")
         self.assertEqual(person1, person2)
 
+    def test_getByEmail_ignores_unvalidated_emails(self):
+        person = self.factory.makePerson()
+        unvalidated_email = self.factory.makeEmail(
+            'fnord@xxxxxxxxxxx',
+            person,
+            email_status=EmailAddressStatus.NEW)
+        found = self.person_set.getByEmail('fnord@xxxxxxxxxxx')
+        self.assertTrue(found is None)
+        
     def test_getPrecachedPersonsFromIDs(self):
         # The getPrecachedPersonsFromIDs() method should only make one
         # query to load all the extraneous data. Accessing the

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-06-02 13:55:16 +0000
+++ lib/lp/registry/vocabularies.py	2012-06-12 22:17:21 +0000
@@ -181,7 +181,10 @@
     shortlist,
     )
 from lp.services.identity.interfaces.account import AccountStatus
-from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
+from lp.services.identity.interfaces.emailaddress import (
+    EmailAddressStatus,
+    VALID_EMAIL_STATUSES,
+    )
 from lp.services.identity.model.account import Account
 from lp.services.identity.model.emailaddress import EmailAddress
 from lp.services.propertycache import (
@@ -234,6 +237,7 @@
             # lookup based on that.
             email = IStore(EmailAddress).find(
                 EmailAddress,
+                EmailAddress.status.is_in(VALID_EMAIL_STATUSES),
                 EmailAddress.email.lower() == token.strip().lower()).one()
             if email is None:
                 raise LookupError(token)
@@ -839,6 +843,7 @@
 
             email_storm_query = self.store.find(
                 EmailAddress.personID,
+                EmailAddress.status.is_in(VALID_EMAIL_STATUSES),
                 EmailAddress.email.lower().startswith(text))
             email_subquery = Alias(email_storm_query._get_select(),
                                    'EmailAddress')

=== modified file 'lib/lp/services/identity/interfaces/emailaddress.py'
--- lib/lp/services/identity/interfaces/emailaddress.py	2012-04-16 23:02:44 +0000
+++ lib/lp/services/identity/interfaces/emailaddress.py	2012-06-12 22:17:21 +0000
@@ -11,7 +11,8 @@
     'EmailAddressStatus',
     'IEmailAddress',
     'IEmailAddressSet',
-    'InvalidEmailAddress']
+    'InvalidEmailAddress',
+    'VALID_EMAIL_STATUSES']
 
 from lazr.enum import (
     DBEnumeratedType,
@@ -86,6 +87,10 @@
         receiving notifications from Launchpad.
         """)
 
+VALID_EMAIL_STATUSES = (
+    EmailAddressStatus.VALIDATED,
+    EmailAddressStatus.PREFERRED)
+
 
 class IEmailAddress(IHasOwner):
     """The object that stores the `IPerson`'s emails."""