Merge lp:~gary-lasker/software-center/recommender-feedback-viewed into lp:software-center

Proposed by Gary Lasker
Status: Merged
Merged at revision: 3165
Proposed branch: lp:~gary-lasker/software-center/recommender-feedback-viewed
Merge into: lp:software-center
Diff against target: 84 lines (+40/-2)
3 files modified
softwarecenter/enums.py (+1/-0)
softwarecenter/ui/gtk3/widgets/recommendations.py (+13/-2)
tests/gtk3/test_catview.py (+26/-0)
To merge this branch: bzr merge lp:~gary-lasker/software-center/recommender-feedback-viewed
Reviewer Review Type Date Requested Status
software-store-developers Pending
Review via email: mp+123670@code.launchpad.net

Commit message

* lp:~gary-lasker/software-center/recommender-feedback-viewed:
    - augment the recommendations feedback to indicate when a
      recommended item has been selected (LP: #1044107)

Description of the change

This branch augments the implicit recommender feature (bug 1044107) by making a call to the recommender service when a recommended item is viewed, rather than only after a successful install. This was added in response to a request from the server team. A unit test for the new call is included.

Many thanks for your review!

To post a comment you must log in.
Michael Vogt (mvo) wrote :

Thanks, that looks good and its great that this could get added quickly (and I like the test!).

Unfortunately the test fails for me with:
$ PYTHONPATH=. python tests/gtk3/test_catview.py RecommendationsTestCase
...
======================================================================
ERROR: test_implicit_recommender_feedback_on_item_viewed (__main__.RecommendationsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gtk3/test_catview.py", line 276, in test_implicit_recommender_feedback_on_item_viewed
    self._populate_recommended_for_you_panel()
AttributeError: 'RecommendationsTestCase' object has no attribute '_populate_recommended_for_you_panel'

======================================================================
FAIL: test_recommended_for_you_spinner_display (__main__.RecommendationsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gtk3/test_catview.py", line 162, in test_recommended_for_you_spinner_display
    self.rec_panel.recommended_for_you_content.get_property("visible"))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 10 tests in 20.199s

One of the failures is probably releated to the lp:~gary-lasker/software-center/recommender-test-fix that I merged earlier and trivial to update. I have no idea about the other one, but it could be something that
is simply a side effect of the first.

When this branch needs to get touched anyway it would be nice to add a small comment to "on_application_activated" like "# send viewed items to the recommender in addition to installed ones" or similar. For a moment I was confused when looking at the code as it looks similar to the part in _on_transaction_finished (i.e. the difference of the two blocks is only visible when looking at the last part that has either "RecommenderFeedbackActions.INSTALLED" or RecommenderFeedbackActions.VIEWED). Or maybe extracting it into a common function?

3159. By Gary Lasker on 2012-09-11

merge trunk

3160. By Gary Lasker on 2012-09-11

fix the new unit test to conform to the updates that were contained in my other (not-yet-merged-at-the-time) branch lp:~gary-lasker/software-center/recommender-test-fix

3161. By Gary Lasker on 2012-09-11

add some comments as requested in the MP

Gary Lasker (gary-lasker) wrote :

Hi Michael, thanks as always for your review. Indeed, the first failure is a trivial case related to my other branch lp:~gary-lasker/software-center/recommender-test-fix. That branch contained a change to a method name (to make its purpose more clear), and as it had not yet been merged to trunk yet my new test still used the previous method name. I merged trunk and fixed this line and the test works again now.

I also added comments to both places where I make implicit_feedback calls to the recommender service.

Finally, the second failure above (test_recommended_for_you_spinner_display) is unrelated to any changes that I have made, and as I mentioned in https://code.launchpad.net/~gary-lasker/software-center/recommender-test-fix/+merge/123669, I have seen this one intermittently for a while now). I think it is simply due to a race issue in the unit test as I see no issues at all in the working code. I'll take a look at that test and see if I can make it reliable, but that work in another branch!

I think this one is ready for review again. Thanks again!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/enums.py'
2--- softwarecenter/enums.py 2012-08-29 22:56:22 +0000
3+++ softwarecenter/enums.py 2012-09-11 14:47:19 +0000
4@@ -284,6 +284,7 @@
5
6 # action values for recommendations feedback
7 class RecommenderFeedbackActions:
8+ VIEWED = "viewed"
9 INSTALLED = "installed"
10
11 # mouse event codes for back/forward buttons
12
13=== modified file 'softwarecenter/ui/gtk3/widgets/recommendations.py'
14--- softwarecenter/ui/gtk3/widgets/recommendations.py 2012-09-05 20:34:14 +0000
15+++ softwarecenter/ui/gtk3/widgets/recommendations.py 2012-09-11 14:47:19 +0000
16@@ -72,10 +72,19 @@
17
18 def _on_application_activated(self, tile, app):
19 self.emit("application-activated", app)
20- # we only track installed items if the user has opted-in to the
21+ # we only report on items if the user has opted-in to the
22 # recommendations service
23 if self.recommender_agent.is_opted_in():
24 self.recommended_apps_viewed.add(app.pkgname)
25+ if network_state_is_connected():
26+ # let the recommendations service know that a
27+ # recommended item has been viewed (if it is
28+ # subsequently installed we will send an additional
29+ # signal to indicate that, in on_transaction_finished)
30+ # (there is no need to monitor for an error, etc., for this)
31+ self.recommender_agent.post_implicit_feedback(
32+ app.pkgname,
33+ RecommenderFeedbackActions.VIEWED)
34
35 def _on_transaction_started(self, backend, pkgname, appname, trans_id,
36 trans_type):
37@@ -89,7 +98,9 @@
38 if result.pkgname in self.recommended_apps_viewed:
39 self.recommended_apps_viewed.remove(result.pkgname)
40 if network_state_is_connected():
41- # no need to monitor for an error, etc., for this
42+ # let the recommendations service know that a
43+ # recommended item has been successfully installed
44+ # (there is no need to monitor for an error, etc., for this)
45 self.recommender_agent.post_implicit_feedback(
46 result.pkgname,
47 RecommenderFeedbackActions.INSTALLED)
48
49=== modified file 'tests/gtk3/test_catview.py'
50--- tests/gtk3/test_catview.py 2012-09-11 12:53:37 +0000
51+++ tests/gtk3/test_catview.py 2012-09-11 14:47:19 +0000
52@@ -271,6 +271,32 @@
53 self.assertFalse(self._app.pkgname in
54 self.rec_panel.recommended_apps_viewed)
55
56+ def test_implicit_recommender_feedback_on_item_viewed(self):
57+ self._opt_in_and_populate_recommended_for_you_panel()
58+ # we fake the callback from the agent here
59+ for_you = self.rec_panel.recommended_for_you_cat
60+ for_you._recommend_me_result(None,
61+ make_recommender_agent_recommend_me_dict())
62+ do_events()
63+
64+ post_implicit_feedback_fn = ('softwarecenter.ui.gtk3.widgets'
65+ '.recommendations.RecommenderAgent'
66+ '.post_implicit_feedback')
67+ with patch(post_implicit_feedback_fn) as mock_post_implicit_feedback:
68+ # we want to grab the app that is activated, it will be in
69+ # self._app after the app is activated on the tile click
70+ self.rec_panel.recommended_for_you_content.connect(
71+ "application-activated",
72+ self._on_application_activated)
73+ # click a recommendation in the lobby
74+ self._click_first_tile_in_panel(self.rec_panel)
75+ # and verify that upon selecting a recommended app we have fired
76+ # the implicit feedback call to the recommender with the correct
77+ # arguments
78+ mock_post_implicit_feedback.assert_called_with(
79+ self._app.pkgname,
80+ RecommenderFeedbackActions.VIEWED)
81+
82 def test_implicit_recommender_feedback_recommendations_panel_only(self):
83 # this test insures that we only provide feedback when installing
84 # items clicked to via the recommendations panel itself, and not

Subscribers

People subscribed via source and target branches