launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-718809 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #718809 New users should default to not receiving email for their own actions
  https://bugs.launchpad.net/bugs/718809

For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-718809/+merge/49950

= Bug 718809 =

Default to not receiving self-generated bug mail for new users.

== Proposed fix ==

Change interface and model column definitions, which basically changes the default.  A separate branch (lp:~danilo/launchpad/db-bug-718809) will change the DB default, but that's mostly to avoid any confusion.

== Implementation details ==

I am sure many other tests which create Persons inline will start failing now, so I'll have to go and fix those as well.  I am hoping the simple change to a call to makePerson will be sufficient.

Lint warning is because of a comment preceding class definition, number of blank lines is correct.

== Tests ==

bin/test -cvvt bugnotification-sending -t selfgenerated_bugnotifications

== Demo and Q/A ==

Register a new account and make sure the checkbox for "Send me bug notifications for changes I make" on https://qastaging.launchpad.net/people/+me/+edit is not checked.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/doc/bugnotification-sending.txt
  lib/lp/testing/factory.py
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_person.py

./lib/lp/registry/interfaces/person.py
     604: E302 expected 2 blank lines, found 1
-- 
https://code.launchpad.net/~danilo/launchpad/bug-718809/+merge/49950
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-718809 into lp:launchpad.

=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt	2011-02-08 14:34:55 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt	2011-02-16 11:30:22 +0000
@@ -877,7 +877,8 @@
     ...             "will be automatically wrapped by the BugNotification "
     ...             "machinery. Ain't technology great?")
     ...     verbose_person = factory.makePerson(
-    ...         displayname='Verbose Person', email='verbose@xxxxxxxxxxx')
+    ...         displayname='Verbose Person', email='verbose@xxxxxxxxxxx',
+    ...         selfgenerated_bugnotifications=True)
     ...     verbose_person.verbose_bugnotifications = True
     ...     ignored = bug.subscribe(verbose_person, verbose_person)
     ...     concise_person = factory.makePerson(
@@ -1099,17 +1100,21 @@
 they generated.  For now, everyone receives these notifications whether they
 want them or not.
 
-    >>> verbose_person.selfgenerated_bugnotifications
-    True
-    >>> with lp_dbuser():
-    ...     verbose_person.selfgenerated_bugnotifications = False
+    >>> with lp_dbuser():
+    ...     person = factory.makePerson()
+    >>> person.selfgenerated_bugnotifications
+    False
+    >>> with lp_dbuser():
+    ...     person.selfgenerated_bugnotifications = True
 
 Teams provide this attribute read-only.
 
-    >>> verbose_team.selfgenerated_bugnotifications
-    True
-    >>> with lp_dbuser():
-    ...     verbose_team.selfgenerated_bugnotifications = False
+    >>> with lp_dbuser():
+    ...     team = factory.makeTeam()
+    >>> team.selfgenerated_bugnotifications
+    False
+    >>> with lp_dbuser():
+    ...     team.selfgenerated_bugnotifications = True
     Traceback (most recent call last):
     ...
     NotImplementedError: Teams do not support changing this attribute.

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-02-07 16:46:35 +0000
+++ lib/lp/registry/interfaces/person.py	2011-02-16 11:30:22 +0000
@@ -633,7 +633,7 @@
 
     selfgenerated_bugnotifications = Bool(
         title=_("Send me bug notifications for changes I make."),
-        required=False, default=True)
+        required=False, default=False)
 
 
 class IPersonPublic(IHasBranches, IHasSpecifications,

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-02-13 15:40:24 +0000
+++ lib/lp/registry/model/person.py	2011-02-16 11:30:22 +0000
@@ -371,7 +371,7 @@
     personID = Int("person", default=None, primary=True)
     person = Reference(personID, "Person.id")
 
-    selfgenerated_bugnotifications = BoolCol(notNull=True, default=True)
+    selfgenerated_bugnotifications = BoolCol(notNull=True, default=False)
 
 
 def readonly_settings(message, interface):

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2011-02-03 08:38:27 +0000
+++ lib/lp/registry/tests/test_person.py	2011-02-16 11:30:22 +0000
@@ -278,6 +278,12 @@
             user.getOwnedOrDrivenPillars()]
         self.assertEqual(expected_pillars, received_pillars)
 
+    def test_selfgenerated_bugnotifications_none_by_default(self):
+        # Default for new accounts is to not get any
+        # self-generated bug notifications by default.
+        user = self.factory.makePerson()
+        self.assertFalse(user.selfgenerated_bugnotifications)
+
 
 class TestPersonStates(TestCaseWithFactory):
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-02-11 19:11:42 +0000
+++ lib/lp/testing/factory.py	2011-02-16 11:30:22 +0000
@@ -547,7 +547,8 @@
     def makePerson(
         self, email=None, name=None, password=None,
         email_address_status=None, hide_email_addresses=False,
-        displayname=None, time_zone=None, latitude=None, longitude=None):
+        displayname=None, time_zone=None, latitude=None, longitude=None,
+        selfgenerated_bugnotifications=False):
         """Create and return a new, arbitrary Person.
 
         :param email: The email address for the new person.
@@ -564,6 +565,7 @@
         :param time_zone: This person's time zone, as a string.
         :param latitude: This person's latitude, as a float.
         :param longitude: This person's longitude, as a float.
+        :param selfgenerated_bugnotifications: Receive own bugmail.
         """
         if email is None:
             email = self.getUniqueEmailAddress()
@@ -594,6 +596,11 @@
         # Make sure the non-security-proxied object is not returned.
         del naked_person
 
+        if selfgenerated_bugnotifications:
+            # Set it explicitely only when True because the default
+            # is False.
+            person.selfgenerated_bugnotifications = True
+
         # To make the person someone valid in Launchpad, validate the
         # email.
         if email_address_status == EmailAddressStatus.PREFERRED: