launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1021198 in Launchpad itself: "With JS off, Specification:+workitems page OOPSes with no workitems"
  https://bugs.launchpad.net/launchpad/+bug/1021198

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

= Bug 1021198: OOPS when editing work items =

On Specification:+workitems, if one clears all the workitems and tries to submit, None is submitted which causes an OOPS.

This fixes it.

I am not fixing all the lint issues since they are unrelated to my branch.  If you want me to do this before landing, please let me know.

== LOC Rationale ==

This is part of https://dev.launchpad.net/Projects/WorkItems

As documented in

  https://docs.google.com/a/linaro.org/spreadsheet/ccc?key=0AvOsYPy8e7yUdGkyRmx2WGFwT3NnSjdHVW04Q1pvSmc

we are still at -298 lines before this branch.

== Tests ==

bin/test -cvvt test_parse_none

== Demo and Q/A ==

Go to any BP and then hack the URL to take you to +workitems page for it (or turn JS off and then click on "edit" icon for workitems).  Clear the entire text area and try to submit.  It shouldn't crash.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/fields/tests/test_fields.py
  lib/lp/services/fields/__init__.py

./lib/lp/services/fields/tests/test_fields.py
     130: E501 line too long (80 characters)
     315: E501 line too long (80 characters)
     366: E501 line too long (80 characters)
     367: W293 blank line contains whitespace
     388: W291 trailing whitespace
     400: W291 trailing whitespace
     424: W291 trailing whitespace
     430: E303 too many blank lines (3)
./lib/lp/services/fields/__init__.py
     937: E501 line too long (85 characters)
-- 
https://code.launchpad.net/~danilo/launchpad/bug-1021198/+merge/113535
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-1021198 into lp:launchpad.

=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py	2012-06-29 08:40:05 +0000
+++ lib/lp/services/fields/__init__.py	2012-07-05 10:28:20 +0000
@@ -892,22 +892,23 @@
         sequence = 0
         milestone = None
         work_items = []
-        for line in text.splitlines():
-            if line.strip() == '':
-                continue
-            milestone_match = MILESTONE_RE.search(line)
-            if milestone_match:
-                milestone_part = milestone_match.group(1).strip()
-                if milestone_part == '':
-                    milestone = None
+        if text is not None:
+            for line in text.splitlines():
+                if line.strip() == '':
+                    continue
+                milestone_match = MILESTONE_RE.search(line)
+                if milestone_match:
+                    milestone_part = milestone_match.group(1).strip()
+                    if milestone_part == '':
+                        milestone = None
+                    else:
+                        milestone = milestone_part.split()[-1]
                 else:
-                    milestone = milestone_part.split()[-1]
-            else:
-                new_work_item = self.parseLine(line)
-                new_work_item['milestone'] = milestone
-                new_work_item['sequence'] = sequence
-                sequence += 1
-                work_items.append(new_work_item)
+                    new_work_item = self.parseLine(line)
+                    new_work_item['milestone'] = milestone
+                    new_work_item['sequence'] = sequence
+                    sequence += 1
+                    work_items.append(new_work_item)
         return work_items
 
     def validate(self, value):

=== modified file 'lib/lp/services/fields/tests/test_fields.py'
--- lib/lp/services/fields/tests/test_fields.py	2012-03-15 07:58:40 +0000
+++ lib/lp/services/fields/tests/test_fields.py	2012-07-05 10:28:20 +0000
@@ -280,6 +280,12 @@
                       'milestone': None,
                       'sequence': 0}])
 
+    def test_parse_none(self):
+        # When empty text box is submitted, None is being passed in instead.
+        work_items_text = None
+        parsed = self.field.parse(work_items_text)
+        self.assertEqual([], parsed)
+
     def test_multi_line_parsing(self):
         title_1 = 'Work item 1'
         title_2 = 'Work item 2'