Code review comment for lp://staging/~barry/launchpad/428957-karma

Revision history for this message
Barry Warsaw (barry) wrote :

 reviewer abentley

= Summary =

Bug 428957 describes a small fallout from my previous landing of the
~person/+karma page conversion. I accidentally displayed
self.user.displayname in the H1 heading instead of self.context.displayname
(i.e. I was displaying the user instead of the person).

== Proposed fix ==

Fix PersonKarmaView.label to return the correct label.

== Pre-implementation notes ==

None. Curtis described the problem and fix exactly in his bug report.

== Implementation details ==

Unfortunately, there were no view tests for +karma, so I added one and tested
the .label attribute directly.

This branch contains a few other sundry fixes. When Salgado reviewed my
original pagetitle branch he made a couple of suggestions in a follow up
comment that I forgot about before I landed the branch. These changes are
found in tales.py:

 * fmt:pagetitle is a final_traversable_names
 * pagetitle() doesn't need its view_name argument

Along the way, I cleaned up a little whitespace and reST-ified a doctest.

== Tests ==

bin/test -vv -t person-karma-views

== Demo and Q/A ==

* Log into launchpad.dev as <email address hidden>:test
* Visit http://launchpad.dev/~no-priv/+karma
* Observe the heading says No Privileges Person
* Visit http://launchpad.dev/~salgado/+karma
* Observe that even logged in as No Priv, you still see Salgado's name in the
  heading.

= Launchpad lint =

Once again, I hate that lint shows up so late in the submission process. I
will fix these lint warnings before I land the branch.

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

Linting changed files:
  lib/lp/registry/doc/person-karma-views.txt
  lib/lp/registry/stories/foaf/xx-person-karma.txt
  lib/lp/registry/doc/person-karma.txt
  lib/canonical/launchpad/webapp/tales.py
  lib/lp/registry/browser/person.py

== Pyflakes Doctest notices ==

lib/lp/registry/doc/person-karma.txt
    22: 'IPerson' imported but unused
    22: 'IKarmaCache' imported but unused

== Pyflakes notices ==

lib/canonical/launchpad/webapp/tales.py
    19: 'warnings' imported but unused

== Pylint notices ==

lib/canonical/launchpad/webapp/tales.py
    23: [F0401] Unable to import 'lazr.enum' (No module named enum)
    19: [W0611] Unused import warnings

lib/lp/registry/browser/person.py
    119: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    120: [F0401] Unable to import 'lazr.config' (No module named config)
    121: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

« Back to merge proposal