Code review comment for lp:~michael.nelson/launchpad/429263-no-value-option

Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =
This branch is a fix for bug 429263.

Since updating the filtering options for pkgs on ppas to use more
zope.formlib stuff, we can no-longer explicitly declare the null-option,
but need to rely on zope's built-in null option for choices (which gets
added irrespectively for non-required choices).

== Proposed fix ==

Remove our explicit 'any' option for status and series filters and
instead rely on zope's built-in None option.

== Pre-implementation notes ==

Discussed briefly with Celso to see how we had handled this in other
situations.

== Implementation details ==

I ended up with two near identical properties - selected_status_filter
and selected_series_filter - hence drying up with the
getSelectedFilterValue() method.

== Tests ==

bin/test -vv -t archive-views -t stories/ppa

== Demo and Q/A ==

There should no longer be a '(no value)' option at:

Local demo, play with filtering at:
https://launchpad.dev/~cprov/+archive/ppa
https://launchpad.dev/~cprov/+archive/ppa/+packages
https://launchpad.dev/~cprov/+archive/ppa/+copy-packages
https://launchpad.dev/~cprov/+archive/ppa/+delete-packages

On edge: similar sub-urls from:

https://edge.launchpad.net/~cprov/+archive/ppa

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.txt
  lib/lp/soyuz/browser/tests/archive-views.txt
  lib/lp/soyuz/stories/ppa/xx-delete-packages.txt
  lib/lp/soyuz/stories/ppa/xx-copy-packages.txt
  lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt
  lib/lp/soyuz/browser/archive.py

--
Michael

« Back to merge proposal