Merge lp:~gary-lasker/software-center/recommended-installed-feedback into lp:software-center
- recommended-installed-feedback
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 3153 |
Proposed branch: | lp:~gary-lasker/software-center/recommended-installed-feedback |
Merge into: | lp:software-center |
Prerequisite: | lp:~gary-lasker/software-center/update-to-latest-recommender-client |
Diff against target: |
942 lines (+356/-133) 11 files modified
softwarecenter/backend/piston/sreclient_pristine.py (+1/-1) softwarecenter/backend/recagent.py (+26/-1) softwarecenter/enums.py (+4/-0) softwarecenter/ui/gtk3/panes/availablepane.py (+3/-10) softwarecenter/ui/gtk3/views/appdetailsview.py (+7/-7) softwarecenter/ui/gtk3/views/catview_gtk.py (+59/-39) softwarecenter/ui/gtk3/widgets/containers.py (+41/-17) softwarecenter/ui/gtk3/widgets/recommendations.py (+79/-29) tests/gtk3/test_catview.py (+117/-21) tests/gtk3/windows.py (+9/-8) tests/test_recagent.py (+10/-0) |
To merge this branch: | bzr merge lp:~gary-lasker/software-center/recommended-installed-feedback |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Vogt | Approve | ||
Review via email: mp+122154@code.launchpad.net |
This proposal supersedes a proposal from 2012-08-28.
Commit message
* lp:~gary-lasker/software-center/recommended-installed-feedback:
- signal the recommender service when a recommended item has been
successfully installed, refactor and clean up surrounding code,
new unit tests for the feature (LP: #944060, LP: #1044107)
Description of the change
This branch implements the "implicit" recommender feedback feature. It detects the case where the user successfully installs a recommended item and fires a "submit_
This is the client-side implementation for bug 944060.
Note that this branch depends on the lp:~gary-lasker/software-center/update-to-latest-recommender-client branch as that branch adds the new recommender client code from lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-recommender-client. Also note that I made one small change to sreclient to fix the validate decorator for the pkgname argument to implicit_feedback so that it matches the validator for the other calls that use pkgname as an argument.
Finally, I added a unit test for the new call in test_recagent.py, however like a few of the other tests there, the test is not currently working with the server. Therefore, it remains disabled as the other tests are. I'll look at this tomorrow to try to see what the issue is with these tests.
Meanwhile, everything here should be working. To test, just select any recommended item in the lobby or category views and install it. Once the installation is complete, the corresponding implicit_feedback call will be fired.
Many thanks for your review!
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal | # |
Gary Lasker (gary-lasker) wrote : Posted in a previous version of this proposal | # |
Just a note that I have a branch for review with the recommender-client changes that I mentioned above (and that have been made to sreclient_
Gary Lasker (gary-lasker) wrote : Posted in a previous version of this proposal | # |
> On Tue, Aug 28, 2012 at 05:46:23AM -0000, Gary Lasker wrote:
> > Gary Lasker has proposed merging lp:~gary-lasker/software-center
> /recommended-
> /software-
>
> Thanks for working on this branch. I'm very happy to see this feature
> coming.
>
> [..]
> > Note that this branch depends on the lp:~gary-lasker/software-center/update-
> to-latest-
> client code from lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-
> recommender-client. Also note that I made one small change to sreclient to fix
> the validate decorator for the pkgname argument to implicit_feedback so that
> it matches the validator for the other calls that use pkgname as an argument.
>
> Thanks for this, if you fixed the sreclient_
> send the diff to the server team (if you haven't done already) so that
> they add it to their
> lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-recommender-client
> branch.
Yep, I proposed a branch for this and it has been merged by Lukasz.
>
> > Finally, I added a unit test for the new call in test_recagent.py, however
> like a few of the other tests there, the test is not currently working with
> the server. Therefore, it remains disabled as the other tests are. I'll look
> at this tomorrow to try to see what the issue is with these tests.
> [..]
>
> Maybe the staging server had problems. I just tested enabling them and
> its working, but that may well be that its now fixed. While looking at
> the test code I noticed that there is a bit of duplication in
> there, I pushed a branch at
> lp:~mvo/software-center/recagent-test-cleanup that removes most of
> that.
>
> I also noticed that the branch is missing a test for the new
> functionality, it would be nice to add one to
> tests/gtk3/
> app, trigger a install transaction and verify that on finish the
> self.recommende
>
I added a thorough test for this functionality in test_catview.py (this is where the other recommendations panel tests are, and imo it's consistent because the tests for the other lobby panels (What's New and Top Rated) are also located here. While in test_catview.py I did some cleanup and consolidation of duplicated code as well.
> > @@ -244,7 +262,7 @@
> >
> > def _on_submit_
> > piston_
> > - self.emit(
> > + self.emit(
> piston_
>
> Nice that this is more consistent now (plus that it will actually work).
>
> > === modified file 'softwarecenter
> > --- softwarecenter/
> > +++ softwarecenter/
> > @@ -299,6 +299,9 @@
> > LOBBY_RECOMMEND
> > DETAILS_
> >
> > +# action values for recommendations implicit f...
Gary Lasker (gary-lasker) wrote : Posted in a previous version of this proposal | # |
Oops, I noticed a typo in my last comment:
"...when a person clicks on a recommended item, we remember that they did that for that item for as long as the current USC session is active (it is now, however, persisted between sessions, I think would be a mistake)."
Should be (see the "*not*"):
"...when a person clicks on a recommended item, we remember that they did that for that item for as long as the current USC session is active (it is *not*, however, persisted between sessions, I think would be a mistake)."
Though maybe this was obvious, but I wanted to make sure that I'm clear.
Thanks again!
Gary Lasker (gary-lasker) wrote : | # |
What the heck? Guess I won't "resubmit" a MP again. :/
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal | # |
On Thu, Aug 30, 2012 at 09:25:21PM -0000, Gary Lasker wrote:
> > On Tue, Aug 28, 2012 at 05:46:23AM -0000, Gary Lasker wrote:
> > > Gary Lasker has proposed merging lp:~gary-lasker/software-center
> > /recommended-
> > /software-
> >
> > Thanks for working on this branch. I'm very happy to see this feature
> > coming.
> >
> > [..]
> > > Note that this branch depends on the lp:~gary-lasker/software-center/update-
> > to-latest-
> > client code from lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-
> > recommender-client. Also note that I made one small change to sreclient to fix
> > the validate decorator for the pkgname argument to implicit_feedback so that
> > it matches the validator for the other calls that use pkgname as an argument.
> >
> > Thanks for this, if you fixed the sreclient_
> > send the diff to the server team (if you haven't done already) so that
> > they add it to their
> > lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-recommender-client
> > branch.
>
> Yep, I proposed a branch for this and it has been merged by Lukasz.
Great, thanks for this.
> > > Finally, I added a unit test for the new call in test_recagent.py, however
> > like a few of the other tests there, the test is not currently working with
> > the server. Therefore, it remains disabled as the other tests are. I'll look
> > at this tomorrow to try to see what the issue is with these tests.
> > [..]
> >
> > Maybe the staging server had problems. I just tested enabling them and
> > its working, but that may well be that its now fixed. While looking at
> > the test code I noticed that there is a bit of duplication in
> > there, I pushed a branch at
> > lp:~mvo/software-center/recagent-test-cleanup that removes most of
> > that.
> >
> > I also noticed that the branch is missing a test for the new
> > functionality, it would be nice to add one to
> > tests/gtk3/
> > app, trigger a install transaction and verify that on finish the
> > self.recommende
> >
>
> I added a thorough test for this functionality in test_catview.py (this is where the other recommendations panel tests are, and imo it's consistent because the tests for the other lobby panels (What's New and Top Rated) are also located here. While in test_catview.py I did some cleanup and consolidation of duplicated code as well.
Thanks for adding the test and for the cleanup there, that looks very
good!
[..]
> > Should self.recommende
> > e.g on _on_app_
> > function for the categories). Or should this be persistent? I'm don't
> > know the details of the spec well enough :) I guess it comes down to
> > if we should consider it a recommendation that are reached
> > "indirectly", e.g. click on it "app A", move back to main view search
> > for something else and then reach "app A" again late...
- 3094. By Gary Lasker
-
add a test to insure that no recommender feedback call occurs in the case where the user is *not* opted in to the recommender service
- 3095. By Gary Lasker
-
merge with trunk
- 3096. By Gary Lasker
-
RED: add a failing test to show that the implicit recommender call is incorrectly being made from the top rated panel
- 3097. By Gary Lasker
-
the fix for the failing test case is to do the refactoring that we know is needed in CategoriesViewGtk to move the add_tiles_
to_flowgrid and corresponding on_application_ selected methods to where they belong, specifically, to the FlowableGrid class itself, this commit contains the brunt of this refactor, but there are some additional pieces needed to finish it up - 3098. By Gary Lasker
-
GREEN: essential refactor complete and signals wired up, all test cases now working; removed the unused (with FIXME) 'application-
selected' signal from catview as it is not used there (we only 'activate' apps from this widget), fix single-line cut-and-paste error in the test case - 3099. By Gary Lasker
-
REFACTOR: make a new TileGrid subclass of FlowableGrid so that FlowableGrid remains generic, wire up the TileGrid for the various panels that it is used
- 3100. By Gary Lasker
-
REFACTOR: remove all need for the catview in the recommendations code
- 3101. By Gary Lasker
-
don't use hasattr in the FramedHeaderBox so that we can attach listeners to the more button when we need to, connect listeners for this, add a signal to the RecommenderPane
lCategory (and subclass RecommenderPane lLobby) so that we can get their correct categories when clicking 'More' in those - 3102. By Gary Lasker
-
pep8 fixes
- 3103. By Gary Lasker
-
merge with trunk
- 3104. By Gary Lasker
-
clean up get_test_
window_ recommendations in windows.py per the new factoring
Gary Lasker (gary-lasker) wrote : | # |
Ok, I've added more unit test cases, done a lot of refactoring and cleanup around this area of the code (lots of FIXMEs cleared) and at this point the branch should be ready for review again. You can review the changes in detail by looking at the checkin comments starting at rev 3094.
Many thanks for your review, as always!!
Michael Vogt (mvo) wrote : | # |
Hi Gary, thanks for this updated branch.
I just reviewed it and it looks very nice now. Some comments below:
I like that the recommendations widgets do no longer get a catview, giving them the db and propertieshelper
looks much nicer now.
395 class FlowableGrid(
396
397 MIN_HEIGHT = 100
398
399 + __gsignals__ = {
400 + "application-
401 + None,
402 + (GObject.
403 + ),
404 + }
405 +
It seems that the FlowableGrid is pretty generic (i.e. can hold other items than application tiles too) so this signal probably is better put into the TileGrid class? Or am I missing something here? I can do this during the merge if you agree with moving it.
I think its great that you added the docstring:
416 + def add_tiles(self, properties_helper, docs, amount):
417 + '''Adds application tiles to an ApplicationTile
418 + properties_help = an instance of the PropertiesHelper object
419 + docs = xapian documents (apps)
420 + amount = number of tiles to add from start of doc range'''
here! It might be worth taking a quick look http://
I like the way the "more" button is now handled, much nicer than in the previous code.
Gary Lasker (gary-lasker) wrote : | # |
> Hi Gary, thanks for this updated branch.
>
> I just reviewed it and it looks very nice now. Some comments below:
>
>
> I like that the recommendations widgets do no longer get a catview, giving
> them the db and propertieshelper
> looks much nicer now.
>
> 395 class FlowableGrid(
> 396
> 397 MIN_HEIGHT = 100
> 398
> 399 + __gsignals__ = {
> 400 + "application-
> 401 + None,
> 402 + (GObject.
> 403 + ),
> 404 + }
> 405 +
>
> It seems that the FlowableGrid is pretty generic (i.e. can hold other items
> than application tiles too) so this signal probably is better put into the
> TileGrid class? Or am I missing something here? I can do this during the merge
> if you agree with moving it.
Yes! Thank you for noticing that. I just overlooked moving it down to the new subclass, but you are correct, that's where it belongs.
>
> I think its great that you added the docstring:
> 416 + def add_tiles(self, properties_helper, docs, amount):
> 417 + '''Adds application tiles to an ApplicationTile
> 418 + properties_help = an instance of the PropertiesHelper object
> 419 + docs = xapian documents (apps)
> 420 + amount = number of tiles to add from start of doc range'''
> here! It might be worth taking a quick look
> http://
> convention that everyone in the python community is following, so to a good
> extend its a matter of taste :) I personally like using a "--" as the keyword
> seperator there, e.g. "doc -- xapian docuemnts" better than using the "=" as
> initially my brain read it as code until I noticed the surrounding '''. But
> thats just a tiny detail and its good to have the docstring.
>
I am with you completely. I just cut and pasted from another docstring and didn't notice the "=" signs. I would much prefer something different, and "--" seems just fine to me.
> I like the way the "more" button is now handled, much nicer than in the
> previous code.
Thanks! I like all of this better too. :) I'll make the updates now.
- 3105. By Gary Lasker
-
move the application-
activated signal into the TileGrid subclass where it belongs - 3106. By Gary Lasker
-
make a docstring more better per review comment
Gary Lasker (gary-lasker) wrote : | # |
Remaining small updates are done in the branch, now we just need the FFe! Bug 1044107. Hopefully very soon now.
- 3107. By Gary Lasker
-
merge with trunk, don't wanna cruft
Preview Diff
1 | === modified file 'softwarecenter/backend/piston/sreclient_pristine.py' |
2 | --- softwarecenter/backend/piston/sreclient_pristine.py 2012-08-28 18:16:47 +0000 |
3 | +++ softwarecenter/backend/piston/sreclient_pristine.py 2012-09-06 15:40:27 +0000 |
4 | @@ -84,7 +84,7 @@ |
5 | scheme=AUTHENTICATED_API_SCHEME) |
6 | |
7 | @oauth_protected |
8 | - @validate('pkgname', str) |
9 | + @validate_pattern('pkgname', '[^/]+') |
10 | @validate_pattern('action', '\w{3,20}') |
11 | @returns_json |
12 | def implicit_feedback(self, pkgname, action): |
13 | |
14 | === modified file 'softwarecenter/backend/recagent.py' |
15 | --- softwarecenter/backend/recagent.py 2012-08-29 18:05:52 +0000 |
16 | +++ softwarecenter/backend/recagent.py 2012-09-06 15:40:27 +0000 |
17 | @@ -68,6 +68,10 @@ |
18 | GObject.TYPE_NONE, |
19 | (GObject.TYPE_PYOBJECT,), |
20 | ), |
21 | + "submit-implicit-feedback-finished": (GObject.SIGNAL_RUN_LAST, |
22 | + GObject.TYPE_NONE, |
23 | + (GObject.TYPE_PYOBJECT,), |
24 | + ), |
25 | "error": (GObject.SIGNAL_RUN_LAST, |
26 | GObject.TYPE_NONE, |
27 | (str,), |
28 | @@ -215,6 +219,22 @@ |
29 | spawner.run_generic_piston_helper( |
30 | "SoftwareCenterRecommenderAPI", "recommend_top") |
31 | |
32 | + def post_implicit_feedback(self, pkgname, action): |
33 | + # build the command |
34 | + LOG.debug("called post_implicit_feedback with %s, '%s'" % |
35 | + (pkgname, action)) |
36 | + spawner = SpawnHelper() |
37 | + spawner.parent_xid = self.xid |
38 | + spawner.needs_auth = True |
39 | + spawner.connect("data-available", |
40 | + self._on_submit_implicit_feedback_data) |
41 | + spawner.connect("error", lambda spawner, err: self.emit("error", err)) |
42 | + spawner.run_generic_piston_helper( |
43 | + "SoftwareCenterRecommenderAPI", |
44 | + "implicit_feedback", |
45 | + pkgname=pkgname, |
46 | + action=action) |
47 | + |
48 | def is_opted_in(self): |
49 | """ |
50 | Return True is the user is currently opted-in to the recommender |
51 | @@ -244,7 +264,7 @@ |
52 | |
53 | def _on_submit_anon_profile_data(self, spawner, |
54 | piston_submit_anon_profile): |
55 | - self.emit("submit-anon_profile", piston_submit_anon_profile) |
56 | + self.emit("submit-anon-profile-finished", piston_submit_anon_profile) |
57 | |
58 | def _on_recommend_me_data(self, spawner, piston_me_apps): |
59 | self.emit("recommend-me", piston_me_apps) |
60 | @@ -258,6 +278,11 @@ |
61 | def _on_recommend_top_data(self, spawner, piston_top_apps): |
62 | self.emit("recommend-top", piston_top_apps) |
63 | |
64 | + def _on_submit_implicit_feedback_data(self, spawner, |
65 | + piston_submit_implicit_feedback): |
66 | + self.emit("submit-implicit-feedback-finished", |
67 | + piston_submit_implicit_feedback) |
68 | + |
69 | def _generate_submit_profile_data(self, recommender_uuid, package_list): |
70 | submit_profile_data = [{ |
71 | 'uuid': recommender_uuid, |
72 | |
73 | === modified file 'softwarecenter/enums.py' |
74 | --- softwarecenter/enums.py 2012-08-23 14:37:28 +0000 |
75 | +++ softwarecenter/enums.py 2012-09-06 15:40:27 +0000 |
76 | @@ -282,6 +282,10 @@ |
77 | PACKAGE = "," |
78 | |
79 | |
80 | +# action values for recommendations feedback |
81 | +class RecommenderFeedbackActions: |
82 | + INSTALLED = "installed" |
83 | + |
84 | # mouse event codes for back/forward buttons |
85 | # TODO: consider whether we ought to get these values from gconf so that we |
86 | # can be sure to use the corresponding values used by Nautilus: |
87 | |
88 | === modified file 'softwarecenter/ui/gtk3/panes/availablepane.py' |
89 | --- softwarecenter/ui/gtk3/panes/availablepane.py 2012-08-21 11:54:39 +0000 |
90 | +++ softwarecenter/ui/gtk3/panes/availablepane.py 2012-09-06 15:40:27 +0000 |
91 | @@ -193,12 +193,7 @@ |
92 | self.subcategories_view.connect( |
93 | "category-selected", self.on_subcategory_activated) |
94 | self.subcategories_view.connect( |
95 | - "application-activated", self.on_application_activated) |
96 | - self.subcategories_view.connect( |
97 | "show-category-applist", self.on_show_category_applist) |
98 | - # FIXME: why do we have two application-{selected,activated] ?!? |
99 | - self.subcategories_view.connect( |
100 | - "application-selected", self.on_application_selected) |
101 | self.subcategories_view.connect( |
102 | "application-activated", self.on_application_activated) |
103 | self.scroll_subcategories = Gtk.ScrolledWindow() |
104 | @@ -215,8 +210,6 @@ |
105 | self.cat_view.connect( |
106 | "category-selected", self.on_category_activated) |
107 | self.cat_view.connect( |
108 | - "application-selected", self.on_application_selected) |
109 | - self.cat_view.connect( |
110 | "application-activated", self.on_application_activated) |
111 | |
112 | # details |
113 | @@ -761,9 +754,9 @@ |
114 | vm = get_viewmanager() |
115 | vm.display_page(self, page, self.state) |
116 | |
117 | - def on_application_selected(self, appview, app): |
118 | - """Callback for when an app is selected.""" |
119 | - super(AvailablePane, self).on_application_selected(appview, app) |
120 | + def on_application_activated(self, appview, app): |
121 | + """Callback for when an app is activated.""" |
122 | + super(AvailablePane, self).on_application_activated(appview, app) |
123 | if self.state.subcategory: |
124 | self.current_app_by_subcategory[self.state.subcategory] = app |
125 | else: |
126 | |
127 | === modified file 'softwarecenter/ui/gtk3/views/appdetailsview.py' |
128 | --- softwarecenter/ui/gtk3/views/appdetailsview.py 2012-08-30 09:04:39 +0000 |
129 | +++ softwarecenter/ui/gtk3/views/appdetailsview.py 2012-09-06 15:40:27 +0000 |
130 | @@ -51,10 +51,6 @@ |
131 | from softwarecenter.backend.weblive import get_weblive_backend |
132 | from softwarecenter.ui.gtk3.dialogs import error |
133 | |
134 | -# FIXME: this is needed for the recommendations but really should become |
135 | -# a widget or something generic instead |
136 | -from softwarecenter.ui.gtk3.views.catview_gtk import CategoriesViewGtk |
137 | - |
138 | from softwarecenter.ui.gtk3.em import StockEms, em |
139 | from softwarecenter.ui.gtk3.drawing import color_to_hex |
140 | from softwarecenter.ui.gtk3.session.appmanager import get_appmanager |
141 | @@ -75,6 +71,8 @@ |
142 | |
143 | import softwarecenter.ui.gtk3.dialogs as dialogs |
144 | |
145 | +from softwarecenter.ui.gtk3.models.appstore2 import AppPropertiesHelper |
146 | + |
147 | from softwarecenter.hw import get_hw_missing_long_description |
148 | from softwarecenter.region import REGION_WARNING_STRING |
149 | |
150 | @@ -822,6 +820,8 @@ |
151 | self.appdetails = None |
152 | self.addons_to_install = [] |
153 | self.addons_to_remove = [] |
154 | + self.properties_helper = AppPropertiesHelper( |
155 | + self.db, self.cache, self.icons) |
156 | # reviews |
157 | self.review_loader = get_review_loader(self.cache, self.db) |
158 | self.review_loader.connect( |
159 | @@ -1301,9 +1301,9 @@ |
160 | vb.pack_start(self._hbars[2], False, False, 0) |
161 | |
162 | # recommendations |
163 | - catview = CategoriesViewGtk( |
164 | - self.datadir, None, self.cache, self.db, self.icons, None) |
165 | - self.recommended_for_app_panel = RecommendationsPanelDetails(catview) |
166 | + self.recommended_for_app_panel = RecommendationsPanelDetails( |
167 | + self.db, |
168 | + self.properties_helper) |
169 | self.recommended_for_app_panel.connect( |
170 | "application-activated", |
171 | self._on_recommended_application_activated) |
172 | |
173 | === modified file 'softwarecenter/ui/gtk3/views/catview_gtk.py' |
174 | --- softwarecenter/ui/gtk3/views/catview_gtk.py 2012-08-22 10:21:06 +0000 |
175 | +++ softwarecenter/ui/gtk3/views/catview_gtk.py 2012-09-06 15:40:27 +0000 |
176 | @@ -39,15 +39,16 @@ |
177 | from softwarecenter.ui.gtk3.models.appstore2 import AppPropertiesHelper |
178 | from softwarecenter.ui.gtk3.widgets.viewport import Viewport |
179 | from softwarecenter.ui.gtk3.widgets.containers import ( |
180 | - FramedHeaderBox, FramedBox, FlowableGrid) |
181 | + FramedHeaderBox, |
182 | + FramedBox, |
183 | + TileGrid) |
184 | from softwarecenter.ui.gtk3.widgets.recommendations import ( |
185 | RecommendationsPanelLobby, |
186 | RecommendationsPanelCategory) |
187 | from softwarecenter.ui.gtk3.widgets.exhibits import ( |
188 | ExhibitBanner, FeaturedExhibit) |
189 | from softwarecenter.ui.gtk3.widgets.buttons import (LabelTile, |
190 | - CategoryTile, |
191 | - FeaturedTile) |
192 | + CategoryTile) |
193 | from softwarecenter.ui.gtk3.em import StockEms |
194 | from softwarecenter.db.appfilter import AppFilter, get_global_filter |
195 | from softwarecenter.db.enquire import AppEnquire |
196 | @@ -74,11 +75,6 @@ |
197 | (GObject.TYPE_PYOBJECT, ), |
198 | ), |
199 | |
200 | - "application-selected": (GObject.SignalFlags.RUN_LAST, |
201 | - None, |
202 | - (GObject.TYPE_PYOBJECT, ), |
203 | - ), |
204 | - |
205 | "application-activated": (GObject.SignalFlags.RUN_LAST, |
206 | None, |
207 | (GObject.TYPE_PYOBJECT, ), |
208 | @@ -154,19 +150,6 @@ |
209 | self.connect("size-allocate", self.on_size_allocate) |
210 | return |
211 | |
212 | - def _add_tiles_to_flowgrid(self, docs, flowgrid, amount): |
213 | - '''Adds application tiles to a FlowableGrid: |
214 | - docs = xapian documents (apps) |
215 | - flowgrid = the FlowableGrid to add tiles to |
216 | - amount = number of tiles to add from start of doc range''' |
217 | - amount = min(len(docs), amount) |
218 | - for doc in docs[0:amount]: |
219 | - tile = FeaturedTile(self.properties_helper, doc) |
220 | - tile.connect('clicked', self.on_app_clicked, |
221 | - self.properties_helper.get_application(doc)) |
222 | - flowgrid.add_child(tile) |
223 | - return |
224 | - |
225 | def on_size_allocate(self, widget, _): |
226 | a = widget.get_allocation() |
227 | prev = self._prev_alloc |
228 | @@ -187,10 +170,11 @@ |
229 | assets["stipple"] = ptrn |
230 | return assets |
231 | |
232 | - def on_app_clicked(self, btn, app): |
233 | - """emit the category-selected signal when a category was clicked""" |
234 | + def on_application_activated(self, btn, app): |
235 | + """ pass the application-activated signal along when an application |
236 | + is clicked-through in one of the tiles |
237 | + """ |
238 | def timeout_emit(): |
239 | - self.emit("application-selected", app) |
240 | self.emit("application-activated", app) |
241 | return False |
242 | |
243 | @@ -362,13 +346,16 @@ |
244 | self.categories, u"Top Rated") # untranslated name |
245 | if top_rated_cat: |
246 | docs = top_rated_cat.get_documents(self.db) |
247 | - self._add_tiles_to_flowgrid(docs, self.top_rated, |
248 | - TOP_RATED_CAROUSEL_LIMIT) |
249 | + self.top_rated.add_tiles(self.properties_helper, |
250 | + docs, |
251 | + TOP_RATED_CAROUSEL_LIMIT) |
252 | self.top_rated.show_all() |
253 | return top_rated_cat |
254 | |
255 | def _append_top_rated(self): |
256 | - self.top_rated = FlowableGrid() |
257 | + self.top_rated = TileGrid() |
258 | + self.top_rated.connect("application-activated", |
259 | + self.on_application_activated) |
260 | #~ self.top_rated.row_spacing = StockEms.SMALL |
261 | self.top_rated_frame = FramedHeaderBox() |
262 | self.top_rated_frame.set_header_label(_("Top Rated")) |
263 | @@ -380,7 +367,6 @@ |
264 | self.top_rated_frame.header_implements_more_button() |
265 | self.top_rated_frame.more.connect('clicked', |
266 | self.on_category_clicked, top_rated_cat) |
267 | - return |
268 | |
269 | def _update_whats_new_content(self): |
270 | # remove any existing children from the grid widget |
271 | @@ -390,13 +376,16 @@ |
272 | self.categories, u"What\u2019s New") # untranslated name |
273 | if whats_new_cat: |
274 | docs = whats_new_cat.get_documents(self.db) |
275 | - self._add_tiles_to_flowgrid(docs, self.whats_new, |
276 | - WHATS_NEW_CAROUSEL_LIMIT) |
277 | + self.whats_new.add_tiles(self.properties_helper, |
278 | + docs, |
279 | + WHATS_NEW_CAROUSEL_LIMIT) |
280 | self.whats_new.show_all() |
281 | return whats_new_cat |
282 | |
283 | def _append_whats_new(self): |
284 | - self.whats_new = FlowableGrid() |
285 | + self.whats_new = TileGrid() |
286 | + self.whats_new.connect("application-activated", |
287 | + self.on_application_activated) |
288 | self.whats_new_frame = FramedHeaderBox() |
289 | self.whats_new_frame.set_header_label(_(u"What\u2019s New")) |
290 | self.whats_new_frame.add(self.whats_new) |
291 | @@ -412,13 +401,33 @@ |
292 | def _update_recommended_for_you_content(self): |
293 | if (self.recommended_for_you_panel and |
294 | self.recommended_for_you_panel.get_parent()): |
295 | + # disconnect listeners |
296 | + self.recommended_for_you_panel.disconnect_by_func( |
297 | + self.on_application_activated) |
298 | + self.recommended_for_you_panel.disconnect_by_func( |
299 | + self.on_category_clicked) |
300 | + # and remove the panel |
301 | self.right_column.remove(self.recommended_for_you_panel) |
302 | - self.recommended_for_you_panel = RecommendationsPanelLobby(self) |
303 | + self.recommended_for_you_panel = RecommendationsPanelLobby( |
304 | + self.db, |
305 | + self.properties_helper) |
306 | + self.recommended_for_you_panel.connect("application-activated", |
307 | + self.on_application_activated) |
308 | + self.recommended_for_you_panel.connect( |
309 | + 'more-button-clicked', |
310 | + self.on_category_clicked) |
311 | self.right_column.pack_start(self.recommended_for_you_panel, |
312 | True, True, 0) |
313 | |
314 | def _append_recommended_for_you(self): |
315 | - self.recommended_for_you_panel = RecommendationsPanelLobby(self) |
316 | + self.recommended_for_you_panel = RecommendationsPanelLobby( |
317 | + self.db, |
318 | + self.properties_helper) |
319 | + self.recommended_for_you_panel.connect("application-activated", |
320 | + self.on_application_activated) |
321 | + self.recommended_for_you_panel.connect( |
322 | + 'more-button-clicked', |
323 | + self.on_category_clicked) |
324 | self.right_column.pack_start( |
325 | self.recommended_for_you_panel, True, True, 0) |
326 | |
327 | @@ -519,12 +528,15 @@ |
328 | 'category': GObject.markup_escape_text(self.header)} |
329 | self.top_rated_frame.set_header_label(m) |
330 | docs = self._get_sub_top_rated_content(category) |
331 | - self._add_tiles_to_flowgrid(docs, self.top_rated, |
332 | - TOP_RATED_CAROUSEL_LIMIT) |
333 | + self.top_rated.add_tiles(self.properties_helper, |
334 | + docs, |
335 | + TOP_RATED_CAROUSEL_LIMIT) |
336 | return |
337 | |
338 | def _append_sub_top_rated(self): |
339 | - self.top_rated = FlowableGrid() |
340 | + self.top_rated = TileGrid() |
341 | + self.top_rated.connect("application-activated", |
342 | + self.on_application_activated) |
343 | self.top_rated.set_row_spacing(6) |
344 | self.top_rated.set_column_spacing(6) |
345 | self.top_rated_frame = FramedHeaderBox() |
346 | @@ -535,10 +547,18 @@ |
347 | def _update_recommended_for_you_in_cat_content(self, category): |
348 | if (self.recommended_for_you_in_cat and |
349 | self.recommended_for_you_in_cat.get_parent()): |
350 | + self.recommended_for_you_in_cat.disconnect_by_func( |
351 | + self.on_application_activated) |
352 | self.vbox.remove(self.recommended_for_you_in_cat) |
353 | self.recommended_for_you_in_cat = RecommendationsPanelCategory( |
354 | - self, |
355 | - category) |
356 | + self.db, |
357 | + self.properties_helper, |
358 | + category) |
359 | + self.recommended_for_you_in_cat.connect("application-activated", |
360 | + self.on_application_activated) |
361 | + self.recommended_for_you_in_cat.connect( |
362 | + 'more-button-clicked', |
363 | + self.on_category_clicked) |
364 | # only show the panel in the categories view when the user |
365 | # is opted in to the recommender service |
366 | # FIXME: this is needed vs. a simple hide() on the widget because |
367 | @@ -596,7 +616,7 @@ |
368 | def _append_subcat_departments(self): |
369 | self.subcat_label = Gtk.Label() |
370 | self.subcat_label.set_alignment(0, 0.5) |
371 | - self.departments = FlowableGrid(paint_grid_pattern=False) |
372 | + self.departments = TileGrid(paint_grid_pattern=False) |
373 | self.departments.set_row_spacing(StockEms.SMALL) |
374 | self.departments.set_column_spacing(StockEms.SMALL) |
375 | self.departments_frame = FramedBox(spacing=StockEms.MEDIUM, |
376 | |
377 | === modified file 'softwarecenter/ui/gtk3/widgets/containers.py' |
378 | --- softwarecenter/ui/gtk3/widgets/containers.py 2012-08-27 20:24:10 +0000 |
379 | +++ softwarecenter/ui/gtk3/widgets/containers.py 2012-09-06 15:40:27 +0000 |
380 | @@ -4,13 +4,14 @@ |
381 | PI_OVER_180 = PI / 180 |
382 | import softwarecenter.paths |
383 | |
384 | -from gi.repository import Gtk, Gdk |
385 | +from gi.repository import Gtk, Gdk, GObject |
386 | |
387 | from buttons import MoreLink |
388 | from softwarecenter.ui.gtk3.em import StockEms |
389 | from softwarecenter.ui.gtk3.drawing import rounded_rect |
390 | from softwarecenter.ui.gtk3.widgets.spinner import (SpinnerView, |
391 | SpinnerNotebook) |
392 | +from softwarecenter.ui.gtk3.widgets.buttons import FeaturedTile |
393 | |
394 | |
395 | class FlowableGrid(Gtk.Fixed): |
396 | @@ -182,6 +183,39 @@ |
397 | self.remove(child) |
398 | |
399 | |
400 | +class TileGrid(FlowableGrid): |
401 | + ''' a flowable layout widget that holds a collection of TileButtons |
402 | + ''' |
403 | + __gsignals__ = { |
404 | + "application-activated": (GObject.SignalFlags.RUN_LAST, |
405 | + None, |
406 | + (GObject.TYPE_PYOBJECT, ), |
407 | + ), |
408 | + } |
409 | + |
410 | + def add_tiles(self, properties_helper, docs, amount): |
411 | + '''Adds application tiles to an ApplicationTileGrid: |
412 | + properties_help -- an instance of the PropertiesHelper object |
413 | + docs -- xapian documents (apps) |
414 | + amount -- number of tiles to add from start of doc range |
415 | + ''' |
416 | + amount = min(len(docs), amount) |
417 | + for doc in docs[0:amount]: |
418 | + tile = FeaturedTile(properties_helper, doc) |
419 | + tile.connect('clicked', self._on_app_clicked, |
420 | + properties_helper.get_application(doc)) |
421 | + self.add_child(tile) |
422 | + |
423 | + # private |
424 | + def _on_app_clicked(self, btn, app): |
425 | + """emit signal when a tile is clicked""" |
426 | + def timeout_emit(): |
427 | + self.emit("application-activated", app) |
428 | + return False |
429 | + |
430 | + GObject.timeout_add(50, timeout_emit) |
431 | + |
432 | + |
433 | # first tier of caching, cache component assets from which frames are |
434 | # rendered |
435 | _frame_asset_cache = {} |
436 | @@ -454,6 +488,9 @@ |
437 | self.spinner_notebook = SpinnerNotebook(self.content_box, |
438 | spinner_size=SpinnerView.SMALL) |
439 | self.box.add(self.spinner_notebook) |
440 | + # make the "More" button, but don't add it to the header unless/until |
441 | + # we get a header_implements_more_button |
442 | + self.more = MoreLink() |
443 | |
444 | def on_draw(self, cr): |
445 | a = self.get_allocation() |
446 | @@ -473,17 +510,6 @@ |
447 | def pack_end(self, *args, **kwargs): |
448 | return self.content_box.pack_end(*args, **kwargs) |
449 | |
450 | - # XXX: non-functional with current code... |
451 | - #~ def set_header_expand(self, expand): |
452 | - #~ alignment = self.header_alignment |
453 | - #~ if expand: |
454 | - #~ expand = 1.0 |
455 | - #~ else: |
456 | - #~ expand = 0.0 |
457 | - #~ alignment.set(alignment.get_property("xalign"), |
458 | - #~ alignment.get_property("yalign"), |
459 | - #~ expand, 1.0) |
460 | - |
461 | def set_header_position(self, position): |
462 | alignment = self.header_alignment |
463 | alignment.set(position, 0.5, |
464 | @@ -502,18 +528,16 @@ |
465 | self.title.set_markup(self.MARKUP % label) |
466 | |
467 | def header_implements_more_button(self): |
468 | - if not hasattr(self, "more"): |
469 | - self.more = MoreLink() |
470 | + if self.more not in self.header: |
471 | self.header.pack_end(self.more, False, False, 0) |
472 | |
473 | def remove_more_button(self): |
474 | - if hasattr(self, "more"): |
475 | + if self.more in self.header: |
476 | self.header.remove(self.more) |
477 | - del self.more |
478 | |
479 | def render_header(self, cr, a, border_radius, assets): |
480 | |
481 | - if hasattr(self, "more"): |
482 | + if self.more in self.header: |
483 | context = self.get_style_context() |
484 | |
485 | # set the arrow fill color |
486 | |
487 | === modified file 'softwarecenter/ui/gtk3/widgets/recommendations.py' |
488 | --- softwarecenter/ui/gtk3/widgets/recommendations.py 2012-08-22 08:16:30 +0000 |
489 | +++ softwarecenter/ui/gtk3/widgets/recommendations.py 2012-09-06 15:40:27 +0000 |
490 | @@ -24,10 +24,11 @@ |
491 | |
492 | from softwarecenter.ui.gtk3.em import StockEms |
493 | from softwarecenter.ui.gtk3.widgets.containers import (FramedHeaderBox, |
494 | - FlowableGrid) |
495 | + TileGrid) |
496 | from softwarecenter.ui.gtk3.utils import get_parent_xid |
497 | from softwarecenter.db.categories import (RecommendedForYouCategory, |
498 | AppRecommendationsCategory) |
499 | +from softwarecenter.backend import get_install_backend |
500 | from softwarecenter.backend.recagent import RecommenderAgent |
501 | from softwarecenter.backend.login_sso import get_sso_backend |
502 | from softwarecenter.backend.ubuntusso import get_ubuntu_sso_backend |
503 | @@ -35,6 +36,8 @@ |
504 | LOBBY_RECOMMENDATIONS_CAROUSEL_LIMIT, |
505 | DETAILS_RECOMMENDATIONS_CAROUSEL_LIMIT, |
506 | SOFTWARE_CENTER_NAME_KEYRING, |
507 | + RecommenderFeedbackActions, |
508 | + TransactionTypes, |
509 | ) |
510 | from softwarecenter.utils import utf8 |
511 | from softwarecenter.netstatus import network_state_is_connected |
512 | @@ -54,18 +57,42 @@ |
513 | ), |
514 | } |
515 | |
516 | - def __init__(self, catview): |
517 | + def __init__(self): |
518 | FramedHeaderBox.__init__(self) |
519 | - # FIXME: we only need the catview for "add_titles_to_flowgrid" |
520 | - # and "on_category_clicked" so we should be able to |
521 | - # extract this to a "leaner" widget |
522 | - self.catview = catview |
523 | - self.catview.connect( |
524 | - "application-activated", self._on_application_activated) |
525 | self.recommender_agent = RecommenderAgent() |
526 | + # keep track of applications that have been viewed via a |
527 | + # recommendation so that we can detect when a recommended app |
528 | + # has been installed |
529 | + self.recommended_apps_viewed = set() |
530 | + self.backend = get_install_backend() |
531 | + self.backend.connect("transaction-started", |
532 | + self._on_transaction_started) |
533 | + self.backend.connect("transaction-finished", |
534 | + self._on_transaction_finished) |
535 | |
536 | - def _on_application_activated(self, catview, app): |
537 | + def _on_application_activated(self, tile, app): |
538 | self.emit("application-activated", app) |
539 | + # we only track installed items if the user has opted-in to the |
540 | + # recommendations service |
541 | + if self.recommender_agent.is_opted_in(): |
542 | + self.recommended_apps_viewed.add(app.pkgname) |
543 | + |
544 | + def _on_transaction_started(self, backend, pkgname, appname, trans_id, |
545 | + trans_type): |
546 | + if (trans_type != TransactionTypes.INSTALL and |
547 | + pkgname in self.recommended_apps_viewed): |
548 | + # if the transaction is not an installation we don't want to |
549 | + # track it as a recommended item |
550 | + self.recommended_apps_viewed.remove(pkgname) |
551 | + |
552 | + def _on_transaction_finished(self, backend, result): |
553 | + if result.pkgname in self.recommended_apps_viewed: |
554 | + self.recommended_apps_viewed.remove(result.pkgname) |
555 | + if network_state_is_connected(): |
556 | + # no need to monitor for an error, etc., for this |
557 | + self.recommender_agent.post_implicit_feedback( |
558 | + result.pkgname, |
559 | + RecommenderFeedbackActions.INSTALLED) |
560 | |
561 | def _on_recommender_agent_error(self, agent, msg): |
562 | LOG.warn("Error while accessing the recommender agent: %s" % msg) |
563 | @@ -81,8 +108,18 @@ |
564 | Panel for use in the category view that displays recommended apps for |
565 | the given category |
566 | """ |
567 | - def __init__(self, catview, subcategory): |
568 | - RecommendationsPanel.__init__(self, catview) |
569 | + |
570 | + __gsignals__ = { |
571 | + "more-button-clicked": (GObject.SignalFlags.RUN_LAST, |
572 | + None, |
573 | + (GObject.TYPE_PYOBJECT, ), |
574 | + ), |
575 | + } |
576 | + |
577 | + def __init__(self, db, properties_helper, subcategory): |
578 | + RecommendationsPanel.__init__(self) |
579 | + self.db = db |
580 | + self.properties_helper = properties_helper |
581 | self.subcategory = subcategory |
582 | if self.subcategory: |
583 | self.set_header_label(GObject.markup_escape_text(utf8( |
584 | @@ -98,7 +135,9 @@ |
585 | if self.recommended_for_you_content: |
586 | self.recommended_for_you_content.destroy() |
587 | # add the new stuff |
588 | - self.recommended_for_you_content = FlowableGrid() |
589 | + self.recommended_for_you_content = TileGrid() |
590 | + self.recommended_for_you_content.connect( |
591 | + "application-activated", self._on_application_activated) |
592 | self.add(self.recommended_for_you_content) |
593 | self.spinner_notebook.show_spinner(_(u"Receiving recommendations…")) |
594 | # get the recommendations from the recommender agent |
595 | @@ -112,23 +151,27 @@ |
596 | |
597 | def _on_recommended_for_you_agent_refresh(self, cat): |
598 | self.header_implements_more_button() |
599 | - docs = cat.get_documents(self.catview.db) |
600 | + self.more.connect("clicked", |
601 | + self._on_more_button_clicked, |
602 | + self.recommended_for_you_cat) |
603 | + docs = cat.get_documents(self.db) |
604 | # display the recommendedations |
605 | if len(docs) > 0: |
606 | - self.catview._add_tiles_to_flowgrid( |
607 | - docs, self.recommended_for_you_content, |
608 | - LOBBY_RECOMMENDATIONS_CAROUSEL_LIMIT) |
609 | + self.recommended_for_you_content.add_tiles( |
610 | + self.properties_helper, |
611 | + docs, |
612 | + LOBBY_RECOMMENDATIONS_CAROUSEL_LIMIT) |
613 | self.recommended_for_you_content.show_all() |
614 | self.spinner_notebook.hide_spinner() |
615 | - self.more.connect('clicked', |
616 | - self.catview.on_category_clicked, |
617 | - cat) |
618 | self.header.queue_draw() |
619 | self.show_all() |
620 | else: |
621 | # hide the panel if we have no recommendations to show |
622 | self.hide() |
623 | |
624 | + def _on_more_button_clicked(self, btn, category): |
625 | + self.emit("more-button-clicked", category) |
626 | + |
627 | |
628 | class RecommendationsPanelLobby(RecommendationsPanelCategory): |
629 | """ |
630 | @@ -155,8 +198,10 @@ |
631 | NO_NETWORK_RECOMMENDATIONS_TEXT = _(u"Recommendations will appear " |
632 | "when next online.") |
633 | |
634 | - def __init__(self, catview): |
635 | - RecommendationsPanel.__init__(self, catview) |
636 | + def __init__(self, db, properties_helper): |
637 | + RecommendationsPanel.__init__(self) |
638 | + self.db = db |
639 | + self.properties_helper = properties_helper |
640 | self.subcategory = None |
641 | self.set_header_label(_(u"Recommended For You")) |
642 | self.recommended_for_you_content = None |
643 | @@ -325,7 +370,7 @@ |
644 | self._on_profile_submitted) |
645 | self.recommender_agent.connect("error", |
646 | self._on_profile_submitted_error) |
647 | - self.recommender_agent.post_submit_profile(self.catview.db) |
648 | + self.recommender_agent.post_submit_profile(self.db) |
649 | |
650 | def _on_profile_submitted(self, agent, profile): |
651 | # after the user profile data has been uploaded, make the request |
652 | @@ -362,10 +407,14 @@ |
653 | Panel for use in the details view to display recommendations for a given |
654 | application |
655 | """ |
656 | - def __init__(self, catview): |
657 | - RecommendationsPanel.__init__(self, catview) |
658 | + def __init__(self, db, properties_helper): |
659 | + RecommendationsPanel.__init__(self) |
660 | + self.db = db |
661 | + self.properties_helper = properties_helper |
662 | self.set_header_label(_(u"People Also Installed")) |
663 | - self.app_recommendations_content = FlowableGrid() |
664 | + self.app_recommendations_content = TileGrid() |
665 | + self.app_recommendations_content.connect( |
666 | + "application-activated", self._on_application_activated) |
667 | self.add(self.app_recommendations_content) |
668 | |
669 | def set_pkgname(self, pkgname): |
670 | @@ -385,12 +434,13 @@ |
671 | self._on_recommender_agent_error) |
672 | |
673 | def _on_app_recommendations_agent_refresh(self, cat): |
674 | - docs = cat.get_documents(self.catview.db) |
675 | + docs = cat.get_documents(self.db) |
676 | # display the recommendations |
677 | if len(docs) > 0: |
678 | - self.catview._add_tiles_to_flowgrid( |
679 | - docs, self.app_recommendations_content, |
680 | - DETAILS_RECOMMENDATIONS_CAROUSEL_LIMIT) |
681 | + self.app_recommendations_content.add_tiles( |
682 | + self.properties_helper, |
683 | + docs, |
684 | + DETAILS_RECOMMENDATIONS_CAROUSEL_LIMIT) |
685 | self.show_all() |
686 | self.spinner_notebook.hide_spinner() |
687 | else: |
688 | |
689 | === modified file 'tests/gtk3/test_catview.py' |
690 | --- tests/gtk3/test_catview.py 2012-08-28 21:20:40 +0000 |
691 | +++ tests/gtk3/test_catview.py 2012-09-06 15:40:27 +0000 |
692 | @@ -20,7 +20,9 @@ |
693 | |
694 | from softwarecenter.db.appfilter import AppFilter |
695 | from softwarecenter.db.database import StoreDatabase |
696 | -from softwarecenter.enums import SortMethods |
697 | +from softwarecenter.enums import (SortMethods, |
698 | + TransactionTypes, |
699 | + RecommenderFeedbackActions) |
700 | from softwarecenter.ui.gtk3.views import catview_gtk |
701 | from softwarecenter.ui.gtk3.widgets.containers import FramedHeaderBox |
702 | from softwarecenter.ui.gtk3.widgets.spinner import SpinnerNotebook |
703 | @@ -35,6 +37,7 @@ |
704 | |
705 | def setUp(self): |
706 | self._cat = None |
707 | + self._app = None |
708 | self.win = get_test_window_catview(self.db) |
709 | self.addCleanup(self.win.destroy) |
710 | self.notebook = self.win.get_child() |
711 | @@ -121,6 +124,7 @@ |
712 | '.post_submit_profile') |
713 | def setUp(self, mock_query, mock_recommender_is_opted_in, mock_sso): |
714 | self._cat = None |
715 | + self._app = None |
716 | # patch the recommender to specify that we are not opted-in at |
717 | # the start of each test |
718 | mock_recommender_is_opted_in.return_value = False |
719 | @@ -162,11 +166,7 @@ |
720 | mock_network_state_is_connected): |
721 | # simulate no network available |
722 | mock_network_state_is_connected.return_value = False |
723 | - # click the opt-in button to initiate the process |
724 | - self.rec_panel.opt_in_button.clicked() |
725 | - do_events() |
726 | - self.rec_panel._update_recommended_for_you_content() |
727 | - do_events() |
728 | + self._populate_recommended_for_you_panel() |
729 | self.assertEqual(self.rec_panel.spinner_notebook.get_current_page(), |
730 | FramedHeaderBox.CONTENT) |
731 | # ensure that we are showing the network not available view |
732 | @@ -176,14 +176,9 @@ |
733 | self.rec_panel.NO_NETWORK_RECOMMENDATIONS_TEXT) |
734 | |
735 | def test_recommended_for_you_display_recommendations(self): |
736 | - # click the opt-in button to initiate the process, |
737 | - # this will show the spinner |
738 | - self.rec_panel.opt_in_button.clicked() |
739 | - do_events() |
740 | - self.rec_panel._update_recommended_for_you_content() |
741 | - do_events() |
742 | + self._populate_recommended_for_you_panel() |
743 | # we fake the callback from the agent here |
744 | - for_you = self.lobby.recommended_for_you_panel.recommended_for_you_cat |
745 | + for_you = self.rec_panel.recommended_for_you_cat |
746 | for_you._recommend_me_result(None, |
747 | make_recommender_agent_recommend_me_dict()) |
748 | self.assertNotEqual(for_you.get_documents(self.db), []) |
749 | @@ -192,7 +187,7 @@ |
750 | do_events() |
751 | # test clicking recommended_for_you More button |
752 | self.lobby.connect("category-selected", self._on_category_selected) |
753 | - self.lobby.recommended_for_you_panel.more.clicked() |
754 | + self.rec_panel.more.clicked() |
755 | # this is delayed for some reason so we need to sleep here |
756 | do_events_with_sleep() |
757 | self.assertNotEqual(self._cat, None) |
758 | @@ -208,13 +203,8 @@ |
759 | self.assertFalse(visible) |
760 | |
761 | def test_recommended_for_you_display_recommendations_opted_in(self): |
762 | - # click the opt-in button to initiate the process, |
763 | - # this will show the spinner |
764 | - self.rec_panel.opt_in_button.clicked() |
765 | - do_events() |
766 | - self.rec_panel._update_recommended_for_you_content() |
767 | - do_events() |
768 | - |
769 | + self._populate_recommended_for_you_panel() |
770 | + |
771 | # we want to work in the "subcat" view |
772 | self.notebook.next_page() |
773 | |
774 | @@ -248,6 +238,112 @@ |
775 | do_events_with_sleep() |
776 | self.assertNotEqual(self._cat, None) |
777 | self.assertEqual(self._cat.name, "Recommended For You in Internet") |
778 | + |
779 | + def test_implicit_recommender_feedback(self): |
780 | + self._populate_recommended_for_you_panel() |
781 | + # we fake the callback from the agent here |
782 | + for_you = self.rec_panel.recommended_for_you_cat |
783 | + for_you._recommend_me_result(None, |
784 | + make_recommender_agent_recommend_me_dict()) |
785 | + do_events() |
786 | + |
787 | + post_implicit_feedback_fn = ('softwarecenter.ui.gtk3.widgets' |
788 | + '.recommendations.RecommenderAgent' |
789 | + '.post_implicit_feedback') |
790 | + with patch(post_implicit_feedback_fn) as mock_post_implicit_feedback: |
791 | + # we want to grab the app that is activated, it will be in |
792 | + # self._app after the app is activated on the tile click |
793 | + self.rec_panel.recommended_for_you_content.connect( |
794 | + "application-activated", |
795 | + self._on_application_activated) |
796 | + # click a recommendation in the lobby |
797 | + self._click_first_tile_in_panel(self.rec_panel) |
798 | + # simulate installing the application |
799 | + self._simulate_install_events(self._app) |
800 | + # and verify that after the install has completed we have fired |
801 | + # the implicit feedback call to the recommender with the correct |
802 | + # arguments |
803 | + mock_post_implicit_feedback.assert_called_with( |
804 | + self._app.pkgname, |
805 | + RecommenderFeedbackActions.INSTALLED) |
806 | + # finally, make sure that we have cleared the application |
807 | + # from the recommended_apps_viewed set |
808 | + self.assertFalse(self._app.pkgname in |
809 | + self.rec_panel.recommended_apps_viewed) |
810 | + |
811 | + def test_implicit_recommender_feedback_recommendations_panel_only(self): |
812 | + # this test insures that we only provide feedback when installing |
813 | + # items clicked to via the recommendations panel itself, and not |
814 | + # via the What's New or Top Rated panels |
815 | + self._populate_recommended_for_you_panel() |
816 | + # we fake the callback from the agent here |
817 | + for_you = self.rec_panel.recommended_for_you_cat |
818 | + for_you._recommend_me_result(None, |
819 | + make_recommender_agent_recommend_me_dict()) |
820 | + do_events() |
821 | + |
822 | + post_implicit_feedback_fn = ('softwarecenter.ui.gtk3.widgets' |
823 | + '.recommendations.RecommenderAgent' |
824 | + '.post_implicit_feedback') |
825 | + with patch(post_implicit_feedback_fn) as mock_post_implicit_feedback: |
826 | + # we want to grab the app that is activated, it will be in |
827 | + # self._app after the app is activated on the tile click |
828 | + self.lobby.top_rated.connect("application-activated", |
829 | + self._on_application_activated) |
830 | + # click a tile in the Top Rated section of the lobby |
831 | + self._click_first_tile_in_panel(self.lobby.top_rated_frame) |
832 | + # simulate installing the application |
833 | + self._simulate_install_events(self._app) |
834 | + # and verify that after the install has completed we have *not* |
835 | + # fired the implicit feedback call to the recommender service |
836 | + self.assertFalse(mock_post_implicit_feedback.called) |
837 | + |
838 | + def test_implicit_recommender_feedback_not_opted_in(self): |
839 | + # this test verifies that we do *not* send feedback to the |
840 | + # recommender service if the user has not opted-in to it |
841 | + post_implicit_feedback_fn = ('softwarecenter.ui.gtk3.widgets' |
842 | + '.recommendations.RecommenderAgent' |
843 | + '.post_implicit_feedback') |
844 | + with patch(post_implicit_feedback_fn) as mock_post_implicit_feedback: |
845 | + # we are not opted-in |
846 | + from softwarecenter.db.application import Application |
847 | + app = Application("Calculator", "gcalctool") |
848 | + # simulate installing the application |
849 | + self._simulate_install_events(app) |
850 | + # and verify that after the install has completed we have *not* |
851 | + # fired the implicit feedback call to the recommender |
852 | + self.assertFalse(mock_post_implicit_feedback.called) |
853 | + |
854 | + def _populate_recommended_for_you_panel(self): |
855 | + # click the opt-in button to initiate the process |
856 | + self.rec_panel.opt_in_button.clicked() |
857 | + do_events() |
858 | + # and update the recommended for you panel to load it up |
859 | + self.rec_panel._update_recommended_for_you_content() |
860 | + do_events() |
861 | + |
862 | + def _on_application_activated(self, catview, app): |
863 | + self._app = app |
864 | + |
865 | + def _click_first_tile_in_panel(self, framed_header_box): |
866 | + first_tile = (framed_header_box.content_box. |
867 | + get_children()[0].get_children()[0]) |
868 | + first_tile.clicked() |
869 | + do_events_with_sleep() |
870 | + |
871 | + def _simulate_install_events(self, app): |
872 | + # pretend we started an install |
873 | + self.rec_panel.backend.emit("transaction-started", |
874 | + app.pkgname, app.appname, |
875 | + "testid101", |
876 | + TransactionTypes.INSTALL) |
877 | + do_events_with_sleep() |
878 | + # send the signal to complete the install |
879 | + mock_result = Mock() |
880 | + mock_result.pkgname = app.pkgname |
881 | + self.rec_panel.backend.emit("transaction-finished", |
882 | + mock_result) |
883 | + do_events() |
884 | |
885 | |
886 | class ExhibitsTestCase(unittest.TestCase): |
887 | |
888 | === modified file 'tests/gtk3/windows.py' |
889 | --- tests/gtk3/windows.py 2012-08-28 21:34:36 +0000 |
890 | +++ tests/gtk3/windows.py 2012-09-06 15:40:27 +0000 |
891 | @@ -368,21 +368,22 @@ |
892 | |
893 | |
894 | def get_test_window_recommendations(panel_type="lobby"): |
895 | - # this is *way* too complicated we should *not* need a CatView |
896 | - # here! see FIXME in RecommendationsPanel.__init__() |
897 | cache = get_test_pkg_info() |
898 | db = get_test_db() |
899 | icons = get_test_gtk3_icon_cache() |
900 | - catview = catview_gtk.CategoriesViewGtk(softwarecenter.paths.datadir, |
901 | - softwarecenter.paths.APP_INSTALL_PATH, cache, db, icons) |
902 | - |
903 | + from softwarecenter.ui.gtk3.models.appstore2 import AppPropertiesHelper |
904 | + properties_helper = AppPropertiesHelper(db, cache, icons) |
905 | + |
906 | if panel_type is "lobby": |
907 | - view = recommendations.RecommendationsPanelLobby(catview) |
908 | + view = recommendations.RecommendationsPanelLobby( |
909 | + db, properties_helper) |
910 | elif panel_type is "category": |
911 | cats = get_test_categories(db) |
912 | - view = recommendations.RecommendationsPanelCategory(catview, cats[0]) |
913 | + view = recommendations.RecommendationsPanelCategory( |
914 | + db, properties_helper, cats[0]) |
915 | else: # panel_type is "details": |
916 | - view = recommendations.RecommendationsPanelDetails(catview) |
917 | + view = recommendations.RecommendationsPanelDetails( |
918 | + db, properties_helper) |
919 | |
920 | win = get_test_window(child=view) |
921 | win.set_data("rec_panel", view) |
922 | |
923 | === modified file 'tests/test_recagent.py' |
924 | --- tests/test_recagent.py 2012-08-29 14:46:51 +0000 |
925 | +++ tests/test_recagent.py 2012-09-06 15:40:27 +0000 |
926 | @@ -129,6 +129,16 @@ |
927 | self.loop.run() |
928 | self.assertTrue(self.error) |
929 | |
930 | + @unittest.skip("server returns 401") |
931 | + def test_recagent_post_implicit_feedback(self): |
932 | + self.recommender_agent.connect("submit-implicit-feedback-finished", |
933 | + self.on_query_done) |
934 | + from softwarecenter.enums import RecommenderFeedbackActions |
935 | + self.recommender_agent.post_implicit_feedback( |
936 | + "bluefish", |
937 | + RecommenderFeedbackActions.INSTALLED) |
938 | + self.assertServerReturnsWithNoError() |
939 | + |
940 | |
941 | if __name__ == "__main__": |
942 | #import logging |
On Tue, Aug 28, 2012 at 05:46:23AM -0000, Gary Lasker wrote:
> Gary Lasker has proposed merging lp:~gary-lasker/software-center/recommended-installed-feedback into lp:software-center with lp:~gary-lasker/software-center/update-to-latest-recommender-client as a prerequisite.
Thanks for working on this branch. I'm very happy to see this feature
coming.
[..]
> Note that this branch depends on the lp:~gary-lasker/software-center/update-to-latest-recommender-client branch as that branch adds the new recommender client code from lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-recommender-client. Also note that I made one small change to sreclient to fix the validate decorator for the pkgname argument to implicit_feedback so that it matches the validator for the other calls that use pkgname as an argument.
Thanks for this, if you fixed the sreclient_ pristine. py, please also
send the diff to the server team (if you haven't done already) so that
they add it to their
lp:~canonical-ca-hackers/ubuntu-recommender/ubuntu-recommender-client
branch.
> Finally, I added a unit test for the new call in test_recagent.py, however like a few of the other tests there, the test is not currently working with the server. Therefore, it remains disabled as the other tests are. I'll look at this tomorrow to try to see what the issue is with these tests.
[..]
Maybe the staging server had problems. I just tested enabling them and
its working, but that may well be that its now fixed. While looking at
the test code I noticed that there is a bit of duplication in
there, I pushed a branch at
lp:~mvo/software-center/recagent-test-cleanup that removes most of
that.
I also noticed that the branch is missing a test for the new test_recommenda tions_widget. py. Something like activate a r_agent. post_implicit_ feedback( ) call was done.
functionality, it would be nice to add one to
tests/gtk3/
app, trigger a install transaction and verify that on finish the
self.recommende
> @@ -244,7 +262,7 @@ anon_profile_ data(self, spawner, submit_ anon_profile) : "submit- anon_profile" , piston_ submit_ anon_profile) "submit- anon-profile- finished" , piston_ submit_ anon_profile)
>
> def _on_submit_
> piston_
> - self.emit(
> + self.emit(
Nice that this is more consistent now (plus that it will actually work).
> === modified file 'softwarecenter /enums. py' enums.py 2012-08-23 14:37:28 +0000 enums.py 2012-08-28 05:45:23 +0000 ATIONS_ CAROUSEL_ LIMIT = 9 RECOMMENDATIONS _CAROUSEL_ LIMIT = 4 S_FEEDBACK_ INSTALLED = "installed"
> --- softwarecenter/
> +++ softwarecenter/
> @@ -299,6 +299,9 @@
> LOBBY_RECOMMEND
> DETAILS_
>
> +# action values for recommendations implicit feedback
> +RECOMMENDATION
> +
That looks like we could use:
class RecommenderFeed back:
INSTALLED = "installed"
(so that later "REMOVED" can be added there as well and its more
consitent with the rest of enums.py).
[..] _activated( self, catview, app): "application- activated" , app) r_agent. i...
> def _on_application
> self.emit(
> + # we only track installed items of the user has opted-in to the
> + # recommendations service
> + if self.recommende