Launchpad itself

Merge lp://staging/~barry/launchpad/428957-karma into lp://staging/launchpad

  1. 428957-karma
  2. Merge into devel
Proposed by Barry Warsaw
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://staging/~barry/launchpad/428957-karma
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~barry/launchpad/428957-karma
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+11700@code.staging.launchpad.net
To post a comment you must log in.
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)

Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 merge approved

Looks sensible to me.

Aaron

Barry Warsaw wrote:
> Barry Warsaw has proposed merging lp:~barry/launchpad/428957-karma into lp:launchpad/devel.
>
> Requested reviews:
> Aaron Bentley (abentley)
>
> 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).

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkquWqYACgkQ0F+nu1YWqI0q9QCeNG8xazofvTXhdIGiC1M3736u
hvUAnROKWFg8hh5ix1wJPjqdGjqLdn30
=pBab
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2009-09-12 01:57:22 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2009-09-14 14:30:02 +0000
@@ -447,12 +447,12 @@
447 traversable_names = {447 traversable_names = {
448 'api_url': 'api_url',448 'api_url': 'api_url',
449 'link': 'link',449 'link': 'link',
450 'url': 'url',
451 }
452
453 # Names which are allowed but can't be traversed further.
454 final_traversable_names = {
450 'pagetitle': 'pagetitle',455 'pagetitle': 'pagetitle',
451 'url': 'url',
452 }
453
454 # Names which are allowed but can't be traversed further.
455 final_traversable_names = {
456 'public-private-css': 'public_private_css',456 'public-private-css': 'public_private_css',
457 }457 }
458458
@@ -544,7 +544,7 @@
544 else:544 else:
545 return 'public'545 return 'public'
546546
547 def pagetitle(self, view_name):547 def pagetitle(self):
548 """The page title to be used.548 """The page title to be used.
549549
550 By default, reverse breadcrumbs are always used if they are available.550 By default, reverse breadcrumbs are always used if they are available.
551551
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2009-09-12 01:57:22 +0000
+++ lib/lp/registry/browser/person.py 2009-09-14 14:10:10 +0000
@@ -2517,7 +2517,7 @@
25172517
2518 @property2518 @property
2519 def label(self):2519 def label(self):
2520 return 'Launchpad Karma for ' + cgi.escape(self.user.displayname)2520 return 'Launchpad Karma for ' + cgi.escape(self.context.displayname)
25212521
2522 @cachedproperty2522 @cachedproperty
2523 def has_karma(self):2523 def has_karma(self):
25242524
=== added file 'lib/lp/registry/doc/person-karma-views.txt'
--- lib/lp/registry/doc/person-karma-views.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/doc/person-karma-views.txt 2009-09-14 14:10:10 +0000
@@ -0,0 +1,21 @@
1=================
2Person karma view
3=================
4
5The ~person/+karma page is controlled by the PersonKarmaView.
6
7 >>> geddy = factory.makePerson(name='geddy', displayname='Geddy Lee')
8 >>> login_person(geddy)
9 >>> view = create_initialized_view(geddy, '+karma')
10
11The view's label shows the person who's karma we're looking at...
12
13 >>> print view.label
14 Launchpad Karma for Geddy Lee
15
16...even when the logged in person is looking at someone else's karma.
17
18 >>> neil = factory.makePerson(name='neil', displayname='Neil Peart')
19 >>> view = create_initialized_view(neil, '+karma')
20 >>> print view.label
21 Launchpad Karma for Neil Peart
022
=== modified file 'lib/lp/registry/doc/person-karma.txt'
--- lib/lp/registry/doc/person-karma.txt 2009-08-31 14:11:15 +0000
+++ lib/lp/registry/doc/person-karma.txt 2009-09-14 14:10:10 +0000
@@ -1,10 +1,12 @@
1= People karma =1============
2People karma
3============
24
3In Launchpad, everytime a user performs an action, we give him some karma5In Launchpad, everytime a user performs an action, we give him some karma
4points. These karma points are stored in the KarmaAction table and the6points. These karma points are stored in the KarmaAction table and the
5assignment to a user is made in the Karma table. The method used to calculate7assignment to a user is made in the Karma table. The method used to calculate
6a users karma is time-dependent, because we want to give more karma points for8a users karma is time-dependent, because we want to give more karma points for
7actions performed recently. This method is described in 9actions performed recently. This method is described in
8https://launchpad.canonical.com/KarmaImplementation.10https://launchpad.canonical.com/KarmaImplementation.
911
10Depending on the action a given person performs in Launchpad, that person can12Depending on the action a given person performs in Launchpad, that person can
@@ -43,7 +45,8 @@
43Salgado wrote the karma framework. Let's give him some karma points.45Salgado wrote the karma framework. Let's give him some karma points.
4446
45 - First, some karma by fixing a bug in firefox47 - First, some karma by fixing a bug in firefox
46 >>> salgado_firefox_karma = salgado.assignKarma('bugfixed', product=firefox)48 >>> salgado_firefox_karma = salgado.assignKarma(
49 ... 'bugfixed', product=firefox)
4750
48 - Then some karma by adding a new spec for Ubuntu51 - Then some karma by adding a new spec for Ubuntu
49 >>> salgado_ubuntu_karma = salgado.assignKarma(52 >>> salgado_ubuntu_karma = salgado.assignKarma(
@@ -79,11 +82,14 @@
79 >>> ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')82 >>> ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')
80 >>> dummy = janitor.assignKarma('specreviewed', product=firefox)83 >>> dummy = janitor.assignKarma('specreviewed', product=firefox)
81 <BLANKLINE>84 <BLANKLINE>
82 85
83== Projects a person is most active on ==86
87Projects a person is most active on
88===================================
8489
85Using our karma records we can also tell in which projects a person is most90Using our karma records we can also tell in which projects a person is most
86active on, including the type of work the person does on each project. We only show the 5 most active projects.91active on, including the type of work the person does on each project. We only
92show the 5 most active projects.
8793
88 >>> foobar = getUtility(IPersonSet).getByName('name16')94 >>> foobar = getUtility(IPersonSet).getByName('name16')
89 >>> for contrib in foobar.getProjectsAndCategoriesContributedTo():95 >>> for contrib in foobar.getProjectsAndCategoriesContributedTo():
@@ -103,7 +109,9 @@
103 [(u'evolution', 175), (u'ubuntu', 26), (u'gnomebaker', 15),109 [(u'evolution', 175), (u'ubuntu', 26), (u'gnomebaker', 15),
104 (u'thunderbird', 15), (u'firefox', 8), (u'bazaar', 2)]110 (u'thunderbird', 15), (u'firefox', 8), (u'bazaar', 2)]
105111
106== Karma Updater ==112
113Karma Updater
114=============
107115
108It would be a problem if every time we wanted to see a user's total karma we116It would be a problem if every time we wanted to see a user's total karma we
109had to calculate it, so we decided to cache this total and update it117had to calculate it, so we decided to cache this total and update it
@@ -143,21 +151,20 @@
143 >>> from canonical.database.sqlbase import flush_database_caches151 >>> from canonical.database.sqlbase import flush_database_caches
144 >>> flush_database_caches()152 >>> flush_database_caches()
145153
146
147Independently of the number of "Bug Management"-related and "Specification154Independently of the number of "Bug Management"-related and "Specification
148Tracking"-related actions performed by Salgado, the total points he gets155Tracking"-related actions performed by Salgado, the total points he gets on
149on each of these categories will always be the same. This is so because156each of these categories will always be the same. This is so because we use a
150we use a scaling factor to balance the total karma of each category and157scaling factor to balance the total karma of each category and because at this
151because at this point, all non-expired karma we have in the database158point, all non-expired karma we have in the database is what we assigned to
152is what we assigned to Salgado during this test. However, when a new159Salgado during this test.
153category is created, its karma pool is dramatically smaller than the160
154existing ones. This causes the scaling to generate rediculous results161However, when a new category is created, its karma pool is dramatically
155until the karma pool starts filling up. To work around this problem,162smaller than the existing ones. This causes the scaling to generate ridiculous
156we ensure that the scaling factors never get too high. So as we saw163results until the karma pool starts filling up. To work around this problem,
157earlier when running the karma updater script, the scaling factor for164we ensure that the scaling factors never get too high. So as we saw earlier
158the Bug Management category was calculated to be 2.667, but reduced to165when running the karma updater script, the scaling factor for the Bug
1592 because this was the maximum specified in166Management category was calculated to be 2.667, but reduced to 2 because this
160config.karmacacheupdater.max_scaling.167was the maximum specified in config.karmacacheupdater.max_scaling.
161168
162 >>> for cache in salgado.karma_category_caches:169 >>> for cache in salgado.karma_category_caches:
163 ... print "%s: %d" % (cache.category.title, cache.karmavalue)170 ... print "%s: %d" % (cache.category.title, cache.karmavalue)
@@ -166,5 +173,3 @@
166173
167 >>> salgado.karma174 >>> salgado.karma
168 70175 70
169
170
171176
=== modified file 'lib/lp/registry/stories/foaf/xx-person-karma.txt'
--- lib/lp/registry/stories/foaf/xx-person-karma.txt 2009-09-12 01:57:22 +0000
+++ lib/lp/registry/stories/foaf/xx-person-karma.txt 2009-09-14 14:10:10 +0000
@@ -1,3 +1,4 @@
1=====
1Karma2Karma
2=====3=====
34