Merge lp:~jmglogow/launchpad/bugfix_420515 into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

Hi Jan.

We normally wait for the developer to propose a merge, but I know your branch fixes the issue and would like to merge it.

[Curtis Hovey (sinzui)]

Hi Jan.

I agree that your branch fixes the most common case, but not all. The field you fixed is in a mixin used by two views. The edit view and the admin view. You could update the admin view to pass the value to createStatusField, but I have an alternate proposal.

Since the status field is required, and the distroseries (eg `context`) is known to the createStatusField method, we do not need to pass the value.

This branch also needs a test. If we had written one years ago we would have known the form was bad.

I think the fix looks something like this:

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2009-11-13 21:55:42 +0000
+++ lib/lp/registry/browser/distroseries.py 2009-11-14 18:26:07 +0000
@@ -272,6 +272,7 @@
         return form.Fields(
             Choice(__name__='status',
                    title=_('Status'),
+ default=self.context.status,
                    vocabulary=status_vocabulary,
                    description=_("Select the distroseries status."),
                    required=True))

=== modified file 'lib/lp/registry/browser/tests/distroseries-views.txt'
--- lib/lp/registry/browser/tests/distroseries-views.txt 2009-09-25 18:15:48 +0000
+++ lib/lp/registry/browser/tests/distroseries-views.txt 2009-11-14 18:23:13 +0000
@@ -186,6 +186,9 @@
     >>> [field.__name__ for field in view.form_fields]
     ['displayname', 'title', 'summary', 'description', 'status']

+ >>> print view.widgets.get('status')._getFormValue()
+ Active Development

I have a branch that revises your changes. If you merge it in I will land your branch
    bzr merge lp:~sinzui/launchpad/bugfix_420515

Thanks a lot for finding a fix for the issue.

review: Needs Fixing (code)

[Curtis Hovey (sinzui)]

I landed the change with the test I added.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2009-11-13 21:55:42 +0000
+++ lib/lp/registry/browser/distroseries.py	2009-11-14 18:30:43 +0000
@@ -245,7 +245,7 @@
 class DistroSeriesStatusMixin:
     """A mixin that provides status field support."""
 
-    def createStatusField(self):
+    def createStatusField(self, default=None):
         """Create the 'status' field.
 
         Create the status vocabulary according the current distroseries
@@ -272,6 +272,7 @@
         return form.Fields(
             Choice(__name__='status',
                    title=_('Status'),
+                   default=default,
                    vocabulary=status_vocabulary,
                    description=_("Select the distroseries status."),
                    required=True))
@@ -369,8 +370,8 @@
         LaunchpadEditFormView.setUpFields(self)
         if not self.context.distribution.full_functionality:
             # This is an IDerivativeDistribution which may set its status.
-            self.form_fields = (
-                self.form_fields + self.createStatusField())
+            self.form_fields = (self.form_fields + 
+                self.createStatusField(self.context.status))
 
     @action("Change")
     def change_action(self, action, data):