launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/proprietary-over-api-mail into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1039630 in Launchpad itself: "Proprietary is permitted in the UI, but not via api and email"
  https://bugs.launchpad.net/launchpad/+bug/1039630

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/proprietary-over-api-mail/+merge/120634

I can see and set bugs to Proprietary information types in the qastaging
UI, but API and email still forbids them.

--------------------------------------------------------------------

RULES

    Pre-implementation: no one
    * In model.Bug, remove from_api kwarg from the method and remove
      the guard.
    * In commands.InformationTypeEmailCommand, remove the guard because
      we want Bug.transitionToInformationType() to enforce policy.


QA

    * Run this API script against qastaging.
      {{{
        def test_bug_sharing_policy():
            lp = Launchpad.login_with(
                'testing',
                service_root='https://api.qastaging.launchpad.net',
                version='devel')
            bug = lp.bugs['939548']
            bug.transitionToInformationType(
                information_type="Proprietary")
      }}}


LINT

    lib/lp/bugs/interfaces/bug.py
    lib/lp/bugs/mail/commands.py
    lib/lp/bugs/mail/tests/test_commands.py
    lib/lp/bugs/model/bug.py
    lib/lp/bugs/model/tests/test_bug.py



TEST

    ./bin/test -vvc lp.bugs.mail.tests.test_commands
    ./bin/test -vvc -t TestBugPriv lp.bugs.model.tests.test_bug


IMPLEMENTATION

Removed the proprietary guard from bug mail commands.
    lib/lp/bugs/mail/commands.py
    lib/lp/bugs/mail/tests/test_commands.py

Removed the proprietary guard from API.
    lib/lp/bugs/interfaces/bug.py
    lib/lp/bugs/model/bug.py
    lib/lp/bugs/model/tests/test_bug.py
-- 
https://code.launchpad.net/~sinzui/launchpad/proprietary-over-api-mail/+merge/120634
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/proprietary-over-api-mail into lp:launchpad.

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2012-08-08 11:04:49 +0000
+++ lib/lp/bugs/interfaces/bug.py	2012-08-21 19:35:23 +0000
@@ -847,10 +847,10 @@
     @operation_parameters(
         information_type=copy_field(IBugPublic['information_type']),
         )
-    @call_with(who=REQUEST_USER, from_api=True)
+    @call_with(who=REQUEST_USER)
     @export_write_operation()
     @operation_for_version("devel")
-    def transitionToInformationType(information_type, who, from_api=False):
+    def transitionToInformationType(information_type, who):
         """Set the information type for this bug.
 
         :information_type: The `InformationType` to transition to.

=== modified file 'lib/lp/bugs/mail/commands.py'
--- lib/lp/bugs/mail/commands.py	2012-08-08 05:36:44 +0000
+++ lib/lp/bugs/mail/commands.py	2012-08-21 19:35:23 +0000
@@ -811,10 +811,6 @@
     def setAttributeValue(self, context, attr_name, attr_value):
         """See EmailCommand."""
         user = getUtility(ILaunchBag).user
-        if attr_value == InformationType.PROPRIETARY:
-            raise EmailProcessingError(
-                'Proprietary bugs are forbidden to be filed via the mail '
-                'interface.')
         if isinstance(context, CreateBugParams):
             context.information_type = attr_value
         else:

=== modified file 'lib/lp/bugs/mail/tests/test_commands.py'
--- lib/lp/bugs/mail/tests/test_commands.py	2012-08-08 11:48:29 +0000
+++ lib/lp/bugs/mail/tests/test_commands.py	2012-08-21 19:35:23 +0000
@@ -451,18 +451,6 @@
         self.assertRaises(
             EmailProcessingError, command.execute, bug_params, dummy_event)
 
-    def test_execute_bug_params_with_proprietary(self):
-        user = self.factory.makePerson()
-        login_person(user)
-        bug_params = CreateBugParams(title='bug title', owner=user)
-        command = InformationTypeEmailCommand(
-            'informationtype', ['proprietary'])
-        dummy_event = object()
-        self.assertRaisesWithContent(
-            EmailProcessingError, 'Proprietary bugs are forbidden to be '
-            'filed via the mail interface.', command.execute, bug_params,
-            dummy_event)
-
 
 class SubscribeEmailCommandTestCase(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-08-15 02:30:46 +0000
+++ lib/lp/bugs/model/bug.py	2012-08-21 19:35:23 +0000
@@ -1709,12 +1709,8 @@
                 set(pillar.getAllowedBugInformationTypes()))
         return types
 
-    def transitionToInformationType(self, information_type, who,
-                                    from_api=False):
+    def transitionToInformationType(self, information_type, who):
         """See `IBug`."""
-        if from_api and information_type == InformationType.PROPRIETARY:
-            raise BugCannotBePrivate(
-                "Cannot transition the information type to proprietary.")
         if self.information_type == information_type:
             return False
         if (information_type == InformationType.PROPRIETARY and

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2012-08-16 05:18:54 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2012-08-21 19:35:23 +0000
@@ -556,16 +556,6 @@
         self.assertThat(
             recorder2, HasQueryCount(Equals(recorder1.count)))
 
-    def test_transitionToInformationType_forbids_proprietary(self):
-        # Calling IBug.transitionToInformationType(PROPRIETARY) over the API
-        # is forbidden currently.
-        bug = self.factory.makeBug()
-        with person_logged_in(bug.owner):
-            self.assertRaisesWithContent(
-                BugCannotBePrivate, "Cannot transition the information type "
-                "to proprietary.", bug.transitionToInformationType,
-                InformationType.PROPRIETARY, bug.owner, True)
-
 
 class TestBugPrivateAndSecurityRelatedUpdatesMixin: