Code review comment for lp:~sinzui/launchpad/noneable-bug-475433

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is my branch to fix the StrippedTextLine field to handle None for
non-required attributes.

    lp:~sinzui/launchpad/noneable-bug-475433
    Diff size: 69
    Launchpad bug: https://bugs.launchpad.net/bugs/475433
    Test command: ./bin/test -vvt "stripped-text-widget"
    Pre-implementation: no one
    Target release: 3.1.10

= Fix the StrippedTextLine field from oopsing =

The OOPS-1405ED465 oops shows the StrippedTextLine is working with None, but
it assumes it has a string.

    * Module canonical.launchpad.fields, line 206, in set
      super(StrippedTextLine, self).set(object, value.strip())
    AttributeError: 'NoneType' object has no attribute 'strip'

The milestone code_name is optional. but is effectively required to avoid
the oops.

The StrippedTextLine.set() method was added to by Tim's 2009-10-2
bmp-inline-commit-message branch to support API operations, but unlike
its counterpart StrippableText.set(), it does not check if the value is None.

== Rules ==

    * StrippedTextLine should test for None before calling strip().

== QA ==

    * Visit a milestone you can edit
    * Set the code name field to empty and submit
    * Verify there is no oops and that the value is not set on the page.

== Lint ==

Linting changed files:
  lib/canonical/launchpad/doc/stripped-text-widget.txt
  lib/canonical/launchpad/fields/__init__.py

== Test ==

    * lib/canonical/launchpad/doc/stripped-text-widget.txt
      * Added a test to verify that the StrippedTextLine strips text and
        can handle None.

== Implementation ==

    * lib/canonical/launchpad/fields/__init__.py
      * Added a guard before calling strip() on the supposed string.
        This is the same approach used by StrippableText for large text.

« Back to merge proposal