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

[Gary Lasker]

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

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!

[Michael Vogt (mvo)]

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?

[lp:~gary-lasker/software-center/recommender-feedback-viewed]

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)]

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!

=== modified file 'softwarecenter/enums.py'
--- softwarecenter/enums.py	2012-08-29 22:56:22 +0000
+++ softwarecenter/enums.py	2012-09-11 14:47:19 +0000
@@ -284,6 +284,7 @@
 
 # action values for recommendations feedback
 class RecommenderFeedbackActions:
+    VIEWED = "viewed"
     INSTALLED = "installed"
 
 # mouse event codes for back/forward buttons
 
=== modified file 'softwarecenter/ui/gtk3/widgets/recommendations.py'
--- softwarecenter/ui/gtk3/widgets/recommendations.py	2012-09-05 20:34:14 +0000
+++ softwarecenter/ui/gtk3/widgets/recommendations.py	2012-09-11 14:47:19 +0000
@@ -72,10 +72,19 @@
 
     def _on_application_activated(self, tile, app):
         self.emit("application-activated", app)
-        # we only track installed items if the user has opted-in to the
+        # we only report on items if the user has opted-in to the
         # recommendations service
         if self.recommender_agent.is_opted_in():
             self.recommended_apps_viewed.add(app.pkgname)
+            if network_state_is_connected():
+                # let the recommendations service know that a
+                # recommended item has been viewed (if it is
+                # subsequently installed we will send an additional
+                # signal to indicate that, in on_transaction_finished)
+                # (there is no need to monitor for an error, etc., for this)
+                self.recommender_agent.post_implicit_feedback(
+                        app.pkgname,
+                        RecommenderFeedbackActions.VIEWED)
 
     def _on_transaction_started(self, backend, pkgname, appname, trans_id,
                                 trans_type):
@@ -89,7 +98,9 @@
         if result.pkgname in self.recommended_apps_viewed:
             self.recommended_apps_viewed.remove(result.pkgname)
             if network_state_is_connected():
-                # no need to monitor for an error, etc., for this
+                # let the recommendations service know that a
+                # recommended item has been successfully installed
+                # (there is no need to monitor for an error, etc., for this)
                 self.recommender_agent.post_implicit_feedback(
                         result.pkgname,
                         RecommenderFeedbackActions.INSTALLED)
 
=== modified file 'tests/gtk3/test_catview.py'
--- tests/gtk3/test_catview.py	2012-09-11 12:53:37 +0000
+++ tests/gtk3/test_catview.py	2012-09-11 14:47:19 +0000
@@ -271,6 +271,32 @@
             self.assertFalse(self._app.pkgname in
                     self.rec_panel.recommended_apps_viewed)
                     
+    def test_implicit_recommender_feedback_on_item_viewed(self):
+        self._opt_in_and_populate_recommended_for_you_panel()
+        # we fake the callback from the agent here
+        for_you = self.rec_panel.recommended_for_you_cat
+        for_you._recommend_me_result(None,
+            make_recommender_agent_recommend_me_dict())
+        do_events()
+        
+        post_implicit_feedback_fn = ('softwarecenter.ui.gtk3.widgets'
+                                     '.recommendations.RecommenderAgent'
+                                     '.post_implicit_feedback')
+        with patch(post_implicit_feedback_fn) as mock_post_implicit_feedback:
+            # we want to grab the app that is activated, it will be in
+            # self._app after the app is activated on the tile click
+            self.rec_panel.recommended_for_you_content.connect(
+                                            "application-activated",
+                                            self._on_application_activated)
+            # click a recommendation in the lobby
+            self._click_first_tile_in_panel(self.rec_panel)
+            # and verify that upon selecting a recommended app we have fired
+            # the implicit feedback call to the recommender with the correct
+            # arguments
+            mock_post_implicit_feedback.assert_called_with(
+                    self._app.pkgname,
+                    RecommenderFeedbackActions.VIEWED)
+                    
     def test_implicit_recommender_feedback_recommendations_panel_only(self):
         # this test insures that we only provide feedback when installing
         # items clicked to via the recommendations panel itself, and not