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
1=== modified file 'lib/canonical/launchpad/webapp/tales.py'
2--- lib/canonical/launchpad/webapp/tales.py 2009-09-12 01:57:22 +0000
3+++ lib/canonical/launchpad/webapp/tales.py 2009-09-14 14:30:02 +0000
4@@ -447,12 +447,12 @@
5 traversable_names = {
6 'api_url': 'api_url',
7 'link': 'link',
8+ 'url': 'url',
9+ }
10+
11+ # Names which are allowed but can't be traversed further.
12+ final_traversable_names = {
13 'pagetitle': 'pagetitle',
14- 'url': 'url',
15- }
16-
17- # Names which are allowed but can't be traversed further.
18- final_traversable_names = {
19 'public-private-css': 'public_private_css',
20 }
21
22@@ -544,7 +544,7 @@
23 else:
24 return 'public'
25
26- def pagetitle(self, view_name):
27+ def pagetitle(self):
28 """The page title to be used.
29
30 By default, reverse breadcrumbs are always used if they are available.
31
32=== modified file 'lib/lp/registry/browser/person.py'
33--- lib/lp/registry/browser/person.py 2009-09-12 01:57:22 +0000
34+++ lib/lp/registry/browser/person.py 2009-09-14 14:10:10 +0000
35@@ -2517,7 +2517,7 @@
36
37 @property
38 def label(self):
39- return 'Launchpad Karma for ' + cgi.escape(self.user.displayname)
40+ return 'Launchpad Karma for ' + cgi.escape(self.context.displayname)
41
42 @cachedproperty
43 def has_karma(self):
44
45=== added file 'lib/lp/registry/doc/person-karma-views.txt'
46--- lib/lp/registry/doc/person-karma-views.txt 1970-01-01 00:00:00 +0000
47+++ lib/lp/registry/doc/person-karma-views.txt 2009-09-14 14:10:10 +0000
48@@ -0,0 +1,21 @@
49+=================
50+Person karma view
51+=================
52+
53+The ~person/+karma page is controlled by the PersonKarmaView.
54+
55+ >>> geddy = factory.makePerson(name='geddy', displayname='Geddy Lee')
56+ >>> login_person(geddy)
57+ >>> view = create_initialized_view(geddy, '+karma')
58+
59+The view's label shows the person who's karma we're looking at...
60+
61+ >>> print view.label
62+ Launchpad Karma for Geddy Lee
63+
64+...even when the logged in person is looking at someone else's karma.
65+
66+ >>> neil = factory.makePerson(name='neil', displayname='Neil Peart')
67+ >>> view = create_initialized_view(neil, '+karma')
68+ >>> print view.label
69+ Launchpad Karma for Neil Peart
70
71=== modified file 'lib/lp/registry/doc/person-karma.txt'
72--- lib/lp/registry/doc/person-karma.txt 2009-08-31 14:11:15 +0000
73+++ lib/lp/registry/doc/person-karma.txt 2009-09-14 14:10:10 +0000
74@@ -1,10 +1,12 @@
75-= People karma =
76+============
77+People karma
78+============
79
80 In Launchpad, everytime a user performs an action, we give him some karma
81 points. These karma points are stored in the KarmaAction table and the
82 assignment to a user is made in the Karma table. The method used to calculate
83 a users karma is time-dependent, because we want to give more karma points for
84-actions performed recently. This method is described in
85+actions performed recently. This method is described in
86 https://launchpad.canonical.com/KarmaImplementation.
87
88 Depending on the action a given person performs in Launchpad, that person can
89@@ -43,7 +45,8 @@
90 Salgado wrote the karma framework. Let's give him some karma points.
91
92 - First, some karma by fixing a bug in firefox
93- >>> salgado_firefox_karma = salgado.assignKarma('bugfixed', product=firefox)
94+ >>> salgado_firefox_karma = salgado.assignKarma(
95+ ... 'bugfixed', product=firefox)
96
97 - Then some karma by adding a new spec for Ubuntu
98 >>> salgado_ubuntu_karma = salgado.assignKarma(
99@@ -79,11 +82,14 @@
100 >>> ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team')
101 >>> dummy = janitor.assignKarma('specreviewed', product=firefox)
102 <BLANKLINE>
103-
104-== Projects a person is most active on ==
105+
106+
107+Projects a person is most active on
108+===================================
109
110 Using our karma records we can also tell in which projects a person is most
111-active on, including the type of work the person does on each project. We only show the 5 most active projects.
112+active on, including the type of work the person does on each project. We only
113+show the 5 most active projects.
114
115 >>> foobar = getUtility(IPersonSet).getByName('name16')
116 >>> for contrib in foobar.getProjectsAndCategoriesContributedTo():
117@@ -103,7 +109,9 @@
118 [(u'evolution', 175), (u'ubuntu', 26), (u'gnomebaker', 15),
119 (u'thunderbird', 15), (u'firefox', 8), (u'bazaar', 2)]
120
121-== Karma Updater ==
122+
123+Karma Updater
124+=============
125
126 It would be a problem if every time we wanted to see a user's total karma we
127 had to calculate it, so we decided to cache this total and update it
128@@ -143,21 +151,20 @@
129 >>> from canonical.database.sqlbase import flush_database_caches
130 >>> flush_database_caches()
131
132-
133 Independently of the number of "Bug Management"-related and "Specification
134-Tracking"-related actions performed by Salgado, the total points he gets
135-on each of these categories will always be the same. This is so because
136-we use a scaling factor to balance the total karma of each category and
137-because at this point, all non-expired karma we have in the database
138-is what we assigned to Salgado during this test. However, when a new
139-category is created, its karma pool is dramatically smaller than the
140-existing ones. This causes the scaling to generate rediculous results
141-until the karma pool starts filling up. To work around this problem,
142-we ensure that the scaling factors never get too high. So as we saw
143-earlier when running the karma updater script, the scaling factor for
144-the Bug Management category was calculated to be 2.667, but reduced to
145-2 because this was the maximum specified in
146-config.karmacacheupdater.max_scaling.
147+Tracking"-related actions performed by Salgado, the total points he gets on
148+each of these categories will always be the same. This is so because we use a
149+scaling factor to balance the total karma of each category and because at this
150+point, all non-expired karma we have in the database is what we assigned to
151+Salgado during this test.
152+
153+However, when a new category is created, its karma pool is dramatically
154+smaller than the existing ones. This causes the scaling to generate ridiculous
155+results until the karma pool starts filling up. To work around this problem,
156+we ensure that the scaling factors never get too high. So as we saw earlier
157+when running the karma updater script, the scaling factor for the Bug
158+Management category was calculated to be 2.667, but reduced to 2 because this
159+was the maximum specified in config.karmacacheupdater.max_scaling.
160
161 >>> for cache in salgado.karma_category_caches:
162 ... print "%s: %d" % (cache.category.title, cache.karmavalue)
163@@ -166,5 +173,3 @@
164
165 >>> salgado.karma
166 70
167-
168-
169
170=== modified file 'lib/lp/registry/stories/foaf/xx-person-karma.txt'
171--- lib/lp/registry/stories/foaf/xx-person-karma.txt 2009-09-12 01:57:22 +0000
172+++ lib/lp/registry/stories/foaf/xx-person-karma.txt 2009-09-14 14:10:10 +0000
173@@ -1,3 +1,4 @@
174+=====
175 Karma
176 =====
177