Ubuntu Software Center

Merge lp:~aaronp/software-center/review-refactor into lp:software-center

  1. review-refactor
  2. Merge into trunk
Proposed by Aaron Peachey
Status: Merged
Merged at revision: 3146
Proposed branch: lp:~aaronp/software-center/review-refactor
Merge into: lp:software-center
Diff against target: 756 lines (+175/-149)
7 files modified
run_against_testing_env.sh (+1/-1)
softwarecenter/backend/reviews/__init__.py (+42/-16)
softwarecenter/backend/reviews/rnr.py (+66/-77)
softwarecenter/ui/gtk3/app.py (+4/-4)
softwarecenter/ui/gtk3/views/appdetailsview.py (+47/-40)
softwarecenter/ui/qml/reviewslist.py (+11/-7)
tests/gtk3/test_appdetailsview.py (+4/-4)
To merge this branch: bzr merge lp:~aaronp/software-center/review-refactor
Reviewer Review Type Date Requested Status
Michael Vogt Approve
Aaron Peachey Needs Resubmitting
Review via email: mp+122035@code.launchpad.net

Description of the change

This branch simply converts the existing callbacks for retrieval of review data and submission/deletion/modification of reviews. to GObject signals (courtesy of mvo) which makes it more consistent with the rest of the code.

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

Hey Aaron, thanks a lot for your work on this!

I think this is good, there is just one issue with the tests
that probably just needs updating:
$ PYTHONPATH=. python tests/gtk3/test_appdetailsview.py
...
======================================================================
ERROR: test_all_duplicate_reviews_keeps_going (__main__.TestAppdetailsView)
AppDetailsView._reviews_ready_callback will fetch another page if
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/mock.py", line 1224, in patched
    return func(*args, **keywargs)
  File "tests/gtk3/test_appdetailsview.py", line 240, in test_all_duplicate_reviews_keeps_going
    application, callback = mock_get_reviews.call_args[0]
ValueError: need more than 1 value to unpack

======================================================================
ERROR: test_no_reviews_returned_attempts_relaxing (__main__.TestAppdetailsView)
AppDetailsView._reviews_ready_callback will attempt to drop the
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/mock.py", line 1224, in patched
    return func(*args, **keywargs)
  File "tests/gtk3/test_appdetailsview.py", line 206, in test_no_reviews_returned_attempts_relaxing
    application, callback = mock_get_reviews.call_args[0]
ValueError: need more than 1 value to unpack

----------------------------------------------------------------------
Ran 23 tests in 135.414s

I will try to get to it later today unless you want to tackle it (which would be very welcome
of course as well :)

Revision history for this message
Aaron Peachey (aaronp) wrote :

Thanks, I'll take a look at this and sort it out. cheers, Aaron.
On Aug 31, 2012 6:48 PM, "Michael Vogt" <email address hidden> wrote:

> Hey Aaron, thanks a lot for your work on this!
>
> I think this is good, there is just one issue with the tests
> that probably just needs updating:
> $ PYTHONPATH=. python tests/gtk3/test_appdetailsview.py
> ...
> ======================================================================
> ERROR: test_all_duplicate_reviews_keeps_going (__main__.TestAppdetailsView)
> AppDetailsView._reviews_ready_callback will fetch another page if
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/usr/lib/python2.7/dist-packages/mock.py", line 1224, in patched
> return func(*args, **keywargs)
> File "tests/gtk3/test_appdetailsview.py", line 240, in
> test_all_duplicate_reviews_keeps_going
> application, callback = mock_get_reviews.call_args[0]
> ValueError: need more than 1 value to unpack
>
> ======================================================================
> ERROR: test_no_reviews_returned_attempts_relaxing
> (__main__.TestAppdetailsView)
> AppDetailsView._reviews_ready_callback will attempt to drop the
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/usr/lib/python2.7/dist-packages/mock.py", line 1224, in patched
> return func(*args, **keywargs)
> File "tests/gtk3/test_appdetailsview.py", line 206, in
> test_no_reviews_returned_attempts_relaxing
> application, callback = mock_get_reviews.call_args[0]
> ValueError: need more than 1 value to unpack
>
> ----------------------------------------------------------------------
> Ran 23 tests in 135.414s
>
> I will try to get to it later today unless you want to tackle it (which
> would be very welcome
> of course as well :)
> --
>
> https://code.launchpad.net/~aaronp/software-center/review-refactor/+merge/122035
> You are the owner of lp:~aaronp/software-center/review-refactor.
>

lp:~aaronp/software-center/review-refactor updated
2834. By Aaron Peachey

test/gtk3/test_appdetailsview.py: amended 2 failing tests that were using review callbacks to now pass using signals

Revision history for this message
Aaron Peachey (aaronp) wrote :

Amended the two failing tests, as they were trying to use the callbacks that no longer exist. They now emit the relevant gobject signals instead and all tests pass.

review: Needs Resubmitting
Revision history for this message
Michael Vogt (mvo) wrote :

Great, thanks a lot!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'run_against_testing_env.sh'
--- run_against_testing_env.sh 2012-03-14 19:53:59 +0000
+++ run_against_testing_env.sh 2012-09-01 11:39:19 +0000
@@ -1,6 +1,6 @@
1#!/bin/sh1#!/bin/sh
22
3export SOFTWARE_CENTER_REVIEWS_HOST="http://184.82.116.62/reviews/api/1.0"3export SOFTWARE_CENTER_REVIEWS_HOST="https://reviews.staging.ubuntu.com/reviews/api/1.0/"
4export SOFTWARE_CENTER_RECOMMENDER_HOST="http://rec.staging.ubuntu.com"4export SOFTWARE_CENTER_RECOMMENDER_HOST="http://rec.staging.ubuntu.com"
55
6# sso6# sso
77
=== modified file 'softwarecenter/backend/reviews/__init__.py'
--- softwarecenter/backend/reviews/__init__.py 2012-05-31 08:52:50 +0000
+++ softwarecenter/backend/reviews/__init__.py 2012-09-01 11:39:19 +0000
@@ -155,6 +155,7 @@
155155
156class Review(object):156class Review(object):
157 """A individual review object """157 """A individual review object """
158
158 def __init__(self, app):159 def __init__(self, app):
159 # a softwarecenter.db.database.Application object160 # a softwarecenter.db.database.Application object
160 self.app = app161 self.app = app
@@ -230,8 +231,31 @@
230 __gsignals__ = {231 __gsignals__ = {
231 "refresh-review-stats-finished": (GObject.SIGNAL_RUN_LAST,232 "refresh-review-stats-finished": (GObject.SIGNAL_RUN_LAST,
232 GObject.TYPE_NONE,233 GObject.TYPE_NONE,
234 # review-stats list
233 (GObject.TYPE_PYOBJECT,),235 (GObject.TYPE_PYOBJECT,),
234 ),236 ),
237 "get-reviews-finished": (GObject.SIGNAL_RUN_LAST,
238 GObject.TYPE_NONE,
239 # application, reviewslist
240 (GObject.TYPE_PYOBJECT,
241 GObject.TYPE_PYOBJECT),
242 ),
243 "remove-review": (GObject.SIGNAL_RUN_LAST,
244 GObject.TYPE_NONE,
245 # app, the review
246 (GObject.TYPE_PYOBJECT, GObject.TYPE_PYOBJECT),
247 ),
248 "replace-review": (GObject.SIGNAL_RUN_LAST,
249 GObject.TYPE_NONE,
250 # app, the review
251 (GObject.TYPE_PYOBJECT, GObject.TYPE_PYOBJECT),
252 ),
253 "update-usefulness-votes": (GObject.SIGNAL_RUN_LAST,
254 GObject.TYPE_NONE,
255 # useful_votes
256 (GObject.TYPE_PYOBJECT, ),
257 ),
258
235 }259 }
236260
237 # cache the ReviewStats261 # cache the ReviewStats
@@ -275,7 +299,7 @@
275 return True299 return True
276 return False300 return False
277301
278 def get_reviews(self, application, callback, page=1, language=None,302 def get_reviews(self, application, page=1, language=None,
279 sort=0, relaxed=False):303 sort=0, relaxed=False):
280 """run callback f(app, review_list)304 """run callback f(app, review_list)
281 with list of review objects for the given305 with list of review objects for the given
@@ -300,7 +324,7 @@
300 except ValueError:324 except ValueError:
301 pass325 pass
302326
303 def refresh_review_stats(self, callback):327 def refresh_review_stats(self):
304 """ get the review statists and call callback when its there """328 """ get the review statists and call callback when its there """
305 pass329 pass
306330
@@ -476,7 +500,7 @@
476 def _random_summary(self):500 def _random_summary(self):
477 return random.choice(self.SUMMARIES)501 return random.choice(self.SUMMARIES)
478502
479 def get_reviews(self, application, callback, page=1, language=None,503 def get_reviews(self, application, page=1, language=None,
480 sort=0, relaxed=False):504 sort=0, relaxed=False):
481 if not application in self._review_stats_cache:505 if not application in self._review_stats_cache:
482 self.get_review_stats(application)506 self.get_review_stats(application)
@@ -497,7 +521,7 @@
497 reviews.append(review)521 reviews.append(review)
498 self._reviews_cache[application] = reviews522 self._reviews_cache[application] = reviews
499 reviews = self._reviews_cache[application]523 reviews = self._reviews_cache[application]
500 callback(application, reviews)524 self.emit("get-reviews-finished", application, reviews)
501525
502 def get_review_stats(self, application):526 def get_review_stats(self, application):
503 if not application in self._review_stats_cache:527 if not application in self._review_stats_cache:
@@ -507,9 +531,9 @@
507 self._review_stats_cache[application] = stat531 self._review_stats_cache[application] = stat
508 return self._review_stats_cache[application]532 return self._review_stats_cache[application]
509533
510 def refresh_review_stats(self, callback):534 def refresh_review_stats(self):
511 review_stats = []535 review_stats = []
512 callback(review_stats)536 self.emit("refresh-review-stats-finished", review_stats)
513537
514538
515class ReviewLoaderFortune(ReviewLoaderFake):539class ReviewLoaderFortune(ReviewLoaderFake):
@@ -657,16 +681,16 @@
657 self._review_stats_cache = {}681 self._review_stats_cache = {}
658 self._reviews_cache = {}682 self._reviews_cache = {}
659683
660 def get_reviews(self, application, callback, page=1, language=None,684 def get_reviews(self, application, page=1, language=None,
661 sort=0, relaxed=False):685 sort=0, relaxed=False):
662 callback(application, [])686 self.emit("get-reviews-finished", application, [])
663687
664 def get_review_stats(self, application):688 def get_review_stats(self, application):
665 pass689 pass
666690
667 def refresh_review_stats(self, callback):691 def refresh_review_stats(self):
668 review_stats = []692 review_stats = []
669 callback(review_stats)693 self.emit("refresh-review-stats-finished", review_stats)
670694
671695
672review_loader = None696review_loader = None
@@ -695,11 +719,11 @@
695 return review_loader719 return review_loader
696720
697if __name__ == "__main__":721if __name__ == "__main__":
698 def callback(app, reviews):722 def callback(loader, app, reviews):
699 print "app callback:"723 print "app callback:"
700 print app, reviews724 print app, reviews
701725
702 def stats_callback(stats):726 def stats_callback(loader, stats):
703 print "stats callback:"727 print "stats callback:"
704 print stats728 print stats
705729
@@ -719,8 +743,10 @@
719 ReviewLoaderSpawningRNRClient743 ReviewLoaderSpawningRNRClient
720 )744 )
721 loader = ReviewLoaderSpawningRNRClient(cache, db)745 loader = ReviewLoaderSpawningRNRClient(cache, db)
722 print loader.refresh_review_stats(stats_callback)746 loader.connect("refresh-review-stats-finished", stats_callback)
723 print loader.get_reviews(app, callback)747 loader.connect("get-reviews-finished", callback)
748 loader.refresh_review_stats()
749 print loader.get_reviews(app)
724750
725 print "\n\n"751 print "\n\n"
726 print "default loader, press ctrl-c for next loader"752 print "default loader, press ctrl-c for next loader"
@@ -732,5 +758,5 @@
732 app = Application("", "2vcard")758 app = Application("", "2vcard")
733 loader = get_review_loader(cache, db)759 loader = get_review_loader(cache, db)
734 loader.refresh_review_stats(stats_callback)760 loader.refresh_review_stats(stats_callback)
735 loader.get_reviews(app, callback)761 loader.get_reviews(app)
736 main.run()762 main.run()
737763
=== modified file 'softwarecenter/backend/reviews/rnr.py'
--- softwarecenter/backend/reviews/rnr.py 2012-03-16 19:16:21 +0000
+++ softwarecenter/backend/reviews/rnr.py 2012-09-01 11:39:19 +0000
@@ -69,10 +69,10 @@
69 self.rnrclient._offline_mode = not network_state_is_connected()69 self.rnrclient._offline_mode = not network_state_is_connected()
7070
71 # reviews71 # reviews
72 def get_reviews(self, translated_app, callback, page=1,72 def get_reviews(self, translated_app, page=1,
73 language=None, sort=0, relaxed=False):73 language=None, sort=0, relaxed=False):
74 """ public api, triggers fetching a review and calls callback74 """ public api, triggers fetching a review and emits
75 when its ready75 get-reviews-finished signal when its ready
76 """76 """
77 # its fine to use the translated appname here, we only submit the77 # its fine to use the translated appname here, we only submit the
78 # pkgname to the server78 # pkgname to the server
@@ -100,7 +100,7 @@
100 origin = "lp-ppa-%s" % ppa.replace("/", "-")100 origin = "lp-ppa-%s" % ppa.replace("/", "-")
101 # if there is no origin, there is nothing to do101 # if there is no origin, there is nothing to do
102 if not origin:102 if not origin:
103 callback(app, [])103 self.emit("get-reviews-finished", app, [])
104 return104 return
105 distroseries = self.distro.get_codename()105 distroseries = self.distro.get_codename()
106 # run the command and add watcher106 # run the command and add watcher
@@ -115,22 +115,21 @@
115 ]115 ]
116 spawn_helper = SpawnHelper()116 spawn_helper = SpawnHelper()
117 spawn_helper.connect(117 spawn_helper.connect(
118 "data-available", self._on_reviews_helper_data, app, callback)118 "data-available", self._on_reviews_helper_data, app)
119 spawn_helper.run(cmd)119 spawn_helper.run(cmd)
120120
121 def _on_reviews_helper_data(self, spawn_helper, piston_reviews, app,121 def _on_reviews_helper_data(self, spawn_helper, piston_reviews, app):
122 callback):
123 # convert into our review objects122 # convert into our review objects
124 reviews = []123 reviews = []
125 for r in piston_reviews:124 for r in piston_reviews:
126 reviews.append(Review.from_piston_mini_client(r))125 reviews.append(Review.from_piston_mini_client(r))
127 # add to our dicts and run callback126 # add to our dicts and emit signal
128 self._reviews[app] = reviews127 self._reviews[app] = reviews
129 callback(app, self._reviews[app])128 self.emit("get-reviews-finished", app, self._reviews[app])
130 return False129 return False
131130
132 # stats131 # stats
133 def refresh_review_stats(self, callback):132 def refresh_review_stats(self):
134 """ public api, refresh the available statistics """133 """ public api, refresh the available statistics """
135 try:134 try:
136 mtime = os.path.getmtime(self.REVIEW_STATS_CACHE_FILE)135 mtime = os.path.getmtime(self.REVIEW_STATS_CACHE_FILE)
@@ -145,8 +144,7 @@
145 #origin = "any"144 #origin = "any"
146 #distroseries = self.distro.get_codename()145 #distroseries = self.distro.get_codename()
147 spawn_helper = SpawnHelper()146 spawn_helper = SpawnHelper()
148 spawn_helper.connect("data-available", self._on_review_stats_data,147 spawn_helper.connect("data-available", self._on_review_stats_data)
149 callback)
150 if days_delta:148 if days_delta:
151 spawn_helper.run_generic_piston_helper(149 spawn_helper.run_generic_piston_helper(
152 "RatingsAndReviewsAPI", "review_stats", days=days_delta)150 "RatingsAndReviewsAPI", "review_stats", days=days_delta)
@@ -154,8 +152,7 @@
154 spawn_helper.run_generic_piston_helper(152 spawn_helper.run_generic_piston_helper(
155 "RatingsAndReviewsAPI", "review_stats")153 "RatingsAndReviewsAPI", "review_stats")
156154
157 def _on_review_stats_data(self, spawn_helper, piston_review_stats,155 def _on_review_stats_data(self, spawn_helper, piston_review_stats):
158 callback):
159 """ process stdout from the helper """156 """ process stdout from the helper """
160 review_stats = self.REVIEW_STATS_CACHE157 review_stats = self.REVIEW_STATS_CACHE
161158
@@ -163,7 +160,7 @@
163 piston_review_stats):160 piston_review_stats):
164 self.REVIEW_STATS_CACHE = {}161 self.REVIEW_STATS_CACHE = {}
165 self.save_review_stats_cache_file()162 self.save_review_stats_cache_file()
166 self.refresh_review_stats(callback)163 self.refresh_review_stats()
167 return164 return
168165
169 # convert to the format that s-c uses166 # convert to the format that s-c uses
@@ -178,7 +175,6 @@
178 s.dampened_rating = calc_dr(s.rating_spread)175 s.dampened_rating = calc_dr(s.rating_spread)
179 review_stats[s.app] = s176 review_stats[s.app] = s
180 self.REVIEW_STATS_CACHE = review_stats177 self.REVIEW_STATS_CACHE = review_stats
181 callback(review_stats)
182 self.emit("refresh-review-stats-finished", review_stats)178 self.emit("refresh-review-stats-finished", review_stats)
183 self.save_review_stats_cache_file()179 self.save_review_stats_cache_file()
184180
@@ -190,10 +186,8 @@
190 return True186 return True
191187
192 # writing new reviews spawns external helper188 # writing new reviews spawns external helper
193 # FIXME: instead of the callback we should add proper gobject signals
194 def spawn_write_new_review_ui(self, translated_app, version, iconname,189 def spawn_write_new_review_ui(self, translated_app, version, iconname,
195 origin, parent_xid, datadir, callback,190 origin, parent_xid, datadir):
196 done_callback=None):
197 """ this spawns the UI for writing a new review and191 """ this spawns the UI for writing a new review and
198 adds it automatically to the reviews DB """192 adds it automatically to the reviews DB """
199 app = translated_app.get_untranslated_app(self.db)193 app = translated_app.get_untranslated_app(self.db)
@@ -210,13 +204,20 @@
210 cmd += ["--appname", utf8(app.appname)]204 cmd += ["--appname", utf8(app.appname)]
211 spawn_helper = SpawnHelper(format="json")205 spawn_helper = SpawnHelper(format="json")
212 spawn_helper.connect(206 spawn_helper.connect(
213 "data-available", self._on_submit_review_data, app, callback)207 "data-available", self._on_submit_review_data, app)
214 if done_callback:208 spawn_helper.connect("exited", self._on_exited_callback, app)
215 spawn_helper.connect("exited", done_callback)209 spawn_helper.connect("error", self._on_error_callback, app)
216 spawn_helper.connect("error", done_callback)
217 spawn_helper.run(cmd)210 spawn_helper.run(cmd)
218211
219 def _on_submit_review_data(self, spawn_helper, review_json, app, callback):212 def _on_exited_callback(self, spawn_helper, return_code, app):
213 # FIXME: send a proper error here instead!
214 self.emit("get-reviews-finished", app, [])
215
216 def _on_error_callback(self, spawn_helper, error_str, app):
217 # FIXME: send a proper error here instead!
218 self.emit("get-reviews-finished", app, [])
219
220 def _on_submit_review_data(self, spawn_helper, review_json, app):
220 """ called when submit_review finished, when the review was send221 """ called when submit_review finished, when the review was send
221 successfully the callback is triggered with the new reviews222 successfully the callback is triggered with the new reviews
222 """223 """
@@ -229,12 +230,12 @@
229 if not app in self._reviews:230 if not app in self._reviews:
230 self._reviews[app] = []231 self._reviews[app] = []
231 self._reviews[app].insert(0, Review.from_piston_mini_client(review))232 self._reviews[app].insert(0, Review.from_piston_mini_client(review))
232 callback(app, self._reviews[app])233 self.emit("get-reviews-finished", app, self._reviews[app])
233234
234 def spawn_report_abuse_ui(self, review_id, parent_xid, datadir, callback):235 def spawn_report_abuse_ui(self, review_id, parent_xid, datadir):
235 """ this spawns the UI for reporting a review as inappropriate236 """ this spawns the UI for reporting a review as inappropriate
236 and adds the review-id to the internal hide list. once the237 and adds the review-id to the internal hide list. once the
237 operation is complete it will call callback with the updated238 operation is complete it will emit remove-review with the updated
238 review list239 review list
239 """240 """
240 cmd = [os.path.join(datadir, RNRApps.REPORT_REVIEW),241 cmd = [os.path.join(datadir, RNRApps.REPORT_REVIEW),
@@ -245,11 +246,10 @@
245 spawn_helper = SpawnHelper("json")246 spawn_helper = SpawnHelper("json")
246 spawn_helper.connect("exited",247 spawn_helper.connect("exited",
247 self._on_report_abuse_finished,248 self._on_report_abuse_finished,
248 review_id, callback)249 review_id)
249 spawn_helper.run(cmd)250 spawn_helper.run(cmd)
250251
251 def _on_report_abuse_finished(self, spawn_helper, exitcode, review_id,252 def _on_report_abuse_finished(self, spawn_helper, exitcode, review_id):
252 callback):
253 """ called when report_absuse finished """253 """ called when report_absuse finished """
254 LOG.debug("hide id %s " % review_id)254 LOG.debug("hide id %s " % review_id)
255 if exitcode == 0:255 if exitcode == 0:
@@ -258,12 +258,11 @@
258 if str(review.id) == str(review_id):258 if str(review.id) == str(review_id):
259 # remove the one we don't want to see anymore259 # remove the one we don't want to see anymore
260 self._reviews[app].remove(review)260 self._reviews[app].remove(review)
261 callback(app, self._reviews[app], None, 'remove',261 self.emit("remove-review", app, review)
262 review)
263 break262 break
264263
265 def spawn_submit_usefulness_ui(self, review_id, is_useful, parent_xid,264 def spawn_submit_usefulness_ui(self, review_id, is_useful, parent_xid,
266 datadir, callback):265 datadir):
267 cmd = [os.path.join(datadir, RNRApps.SUBMIT_USEFULNESS),266 cmd = [os.path.join(datadir, RNRApps.SUBMIT_USEFULNESS),
268 "--review-id", "%s" % review_id,267 "--review-id", "%s" % review_id,
269 "--is-useful", "%s" % int(is_useful),268 "--is-useful", "%s" % int(is_useful),
@@ -273,21 +272,20 @@
273 spawn_helper = SpawnHelper(format="none")272 spawn_helper = SpawnHelper(format="none")
274 spawn_helper.connect("exited",273 spawn_helper.connect("exited",
275 self._on_submit_usefulness_finished,274 self._on_submit_usefulness_finished,
276 review_id, is_useful, callback)275 review_id, is_useful)
277 spawn_helper.connect("error",276 spawn_helper.connect("error",
278 self._on_submit_usefulness_error,277 self._on_submit_usefulness_error,
279 review_id, callback)278 review_id)
280 spawn_helper.run(cmd)279 spawn_helper.run(cmd)
281280
282 def _on_submit_usefulness_finished(self, spawn_helper, res, review_id,281 def _on_submit_usefulness_finished(self, spawn_helper, res, review_id,
283 is_useful, callback):282 is_useful):
284 """ called when report_usefulness finished """283 """ called when report_usefulness finished """
285 # "Created", "Updated", "Not modified" -284 # "Created", "Updated", "Not modified" -
286 # once lp:~mvo/rnr-server/submit-usefulness-result-strings makes it285 # once lp:~mvo/rnr-server/submit-usefulness-result-strings makes it
287 response = spawn_helper._stdout286 response = spawn_helper._stdout
288 if response == '"Not modified"':287 if response == '"Not modified"':
289 self._on_submit_usefulness_error(spawn_helper, response, review_id,288 self._on_submit_usefulness_error(spawn_helper, response, review_id)
290 callback)
291 return289 return
292290
293 LOG.debug("usefulness id %s " % review_id)291 LOG.debug("usefulness id %s " % review_id)
@@ -301,24 +299,23 @@
301 review.usefulness_total = getattr(review,299 review.usefulness_total = getattr(review,
302 "usefulness_total", 0) + 1300 "usefulness_total", 0) + 1
303 if is_useful:301 if is_useful:
304 review.usefulness_favorable = getattr(review,302 review.usefulness_favorable = getattr(
305 "usefulness_favorable", 0) + 1303 review, "usefulness_favorable", 0) + 1
306 callback(app, self._reviews[app], useful_votes,304 self.emit("update-usefulness-votes", useful_votes)
307 'replace', review)305 self.emit("replace-review", app, review)
308 break306 break
309307
310 def _on_submit_usefulness_error(self, spawn_helper, error_str, review_id,308 def _on_submit_usefulness_error(self, spawn_helper, error_str, review_id):
311 callback):309 LOG.warn("submit usefulness id=%s failed with error: %s" %
312 LOG.warn("submit usefulness id=%s failed with error: %s" %310 (review_id, error_str))
313 (review_id, error_str))311 for (app, reviews) in self._reviews.items():
314 for (app, reviews) in self._reviews.items():312 for review in reviews:
315 for review in reviews:313 if str(review.id) == str(review_id):
316 if str(review.id) == str(review_id):314 review.usefulness_submit_error = True
317 review.usefulness_submit_error = True315 self.emit("replace-review", app, review)
318 callback(app, self._reviews[app], None, 'replace', review)316 break
319 break317
320318 def spawn_delete_review_ui(self, review_id, parent_xid, datadir):
321 def spawn_delete_review_ui(self, review_id, parent_xid, datadir, callback):
322 cmd = [os.path.join(datadir, RNRApps.DELETE_REVIEW),319 cmd = [os.path.join(datadir, RNRApps.DELETE_REVIEW),
323 "--review-id", "%s" % review_id,320 "--review-id", "%s" % review_id,
324 "--parent-xid", "%s" % parent_xid,321 "--parent-xid", "%s" % parent_xid,
@@ -327,13 +324,12 @@
327 spawn_helper = SpawnHelper(format="none")324 spawn_helper = SpawnHelper(format="none")
328 spawn_helper.connect("exited",325 spawn_helper.connect("exited",
329 self._on_delete_review_finished,326 self._on_delete_review_finished,
330 review_id, callback)327 review_id)
331 spawn_helper.connect("error", self._on_delete_review_error,328 spawn_helper.connect("error", self._on_delete_review_error,
332 review_id, callback)329 review_id)
333 spawn_helper.run(cmd)330 spawn_helper.run(cmd)
334331
335 def _on_delete_review_finished(self, spawn_helper, res, review_id,332 def _on_delete_review_finished(self, spawn_helper, res, review_id):
336 callback):
337 """ called when delete_review finished"""333 """ called when delete_review finished"""
338 LOG.debug("delete id %s " % review_id)334 LOG.debug("delete id %s " % review_id)
339 for (app, reviews) in self._reviews.items():335 for (app, reviews) in self._reviews.items():
@@ -341,11 +337,10 @@
341 if str(review.id) == str(review_id):337 if str(review.id) == str(review_id):
342 # remove the one we don't want to see anymore338 # remove the one we don't want to see anymore
343 self._reviews[app].remove(review)339 self._reviews[app].remove(review)
344 callback(app, self._reviews[app], None, 'remove', review)340 self.emit("remove-review", app, review)
345 break341 break
346342
347 def _on_delete_review_error(self, spawn_helper, error_str, review_id,343 def _on_delete_review_error(self, spawn_helper, error_str, review_id):
348 callback):
349 """called if delete review errors"""344 """called if delete review errors"""
350 LOG.warn("delete review id=%s failed with error: %s" % (review_id,345 LOG.warn("delete review id=%s failed with error: %s" % (review_id,
351 error_str))346 error_str))
@@ -353,12 +348,10 @@
353 for review in reviews:348 for review in reviews:
354 if str(review.id) == str(review_id):349 if str(review.id) == str(review_id):
355 review.delete_error = True350 review.delete_error = True
356 callback(app, self._reviews[app], action='replace',351 self.emit("remove-review", app, review)
357 single_review=review)
358 break352 break
359353
360 def spawn_modify_review_ui(self, parent_xid, iconname, datadir, review_id,354 def spawn_modify_review_ui(self, parent_xid, iconname, datadir, review_id):
361 callback):
362 """ this spawns the UI for writing a new review and355 """ this spawns the UI for writing a new review and
363 adds it automatically to the reviews DB """356 adds it automatically to the reviews DB """
364 cmd = [os.path.join(datadir, RNRApps.MODIFY_REVIEW),357 cmd = [os.path.join(datadir, RNRApps.MODIFY_REVIEW),
@@ -370,13 +363,12 @@
370 spawn_helper = SpawnHelper(format="json")363 spawn_helper = SpawnHelper(format="json")
371 spawn_helper.connect("data-available",364 spawn_helper.connect("data-available",
372 self._on_modify_review_finished,365 self._on_modify_review_finished,
373 review_id, callback)366 review_id)
374 spawn_helper.connect("error", self._on_modify_review_error,367 spawn_helper.connect("error", self._on_modify_review_error,
375 review_id, callback)368 review_id)
376 spawn_helper.run(cmd)369 spawn_helper.run(cmd)
377370
378 def _on_modify_review_finished(self, spawn_helper, review_json, review_id,371 def _on_modify_review_finished(self, spawn_helper, review_json, review_id):
379 callback):
380 """called when modify_review finished"""372 """called when modify_review finished"""
381 LOG.debug("_on_modify_review_finished")373 LOG.debug("_on_modify_review_finished")
382 #review_json = spawn_helper._stdout374 #review_json = spawn_helper._stdout
@@ -388,12 +380,10 @@
388 self._reviews[app].remove(review)380 self._reviews[app].remove(review)
389 new_review = Review.from_piston_mini_client(mod_review)381 new_review = Review.from_piston_mini_client(mod_review)
390 self._reviews[app].insert(0, new_review)382 self._reviews[app].insert(0, new_review)
391 callback(app, self._reviews[app], action='replace',383 self.emit("replace-review", app, new_review)
392 single_review=new_review)
393 break384 break
394385
395 def _on_modify_review_error(self, spawn_helper, error_str, review_id,386 def _on_modify_review_error(self, spawn_helper, error_str, review_id):
396 callback):
397 """called if modify review errors"""387 """called if modify review errors"""
398 LOG.debug("modify review id=%s failed with error: %s" %388 LOG.debug("modify review id=%s failed with error: %s" %
399 (review_id, error_str))389 (review_id, error_str))
@@ -401,6 +391,5 @@
401 for review in reviews:391 for review in reviews:
402 if str(review.id) == str(review_id):392 if str(review.id) == str(review_id):
403 review.modify_error = True393 review.modify_error = True
404 callback(app, self._reviews[app], action='replace',394 self.emit("replace-review", app, review)
405 single_review=review)
406 break395 break
407396
=== modified file 'softwarecenter/ui/gtk3/app.py'
--- softwarecenter/ui/gtk3/app.py 2012-08-30 12:13:13 +0000
+++ softwarecenter/ui/gtk3/app.py 2012-09-01 11:39:19 +0000
@@ -414,9 +414,9 @@
414 # reviews414 # reviews
415 with ExecutionTime("create review loader"):415 with ExecutionTime("create review loader"):
416 self.review_loader = get_review_loader(self.cache, self.db)416 self.review_loader = get_review_loader(self.cache, self.db)
417 # FIXME: add some kind of throttle, I-M-S here417 self.review_loader.connect(
418 self.review_loader.refresh_review_stats(418 "refresh-review-stats-finished", self.on_review_stats_loaded)
419 self.on_review_stats_loaded)419 self.review_loader.refresh_review_stats()
420 #load usefulness votes from server when app starts420 #load usefulness votes from server when app starts
421 self.useful_cache = UsefulnessCache(True)421 self.useful_cache = UsefulnessCache(True)
422 self.setup_database_rebuilding_listener()422 self.setup_database_rebuilding_listener()
@@ -603,7 +603,7 @@
603 if os.WEXITSTATUS(condition) == 0:603 if os.WEXITSTATUS(condition) == 0:
604 self.db.reopen()604 self.db.reopen()
605605
606 def on_review_stats_loaded(self, reviews):606 def on_review_stats_loaded(self, loader, reviews):
607 LOG.debug("on_review_stats_loaded: '%s'" % len(reviews))607 LOG.debug("on_review_stats_loaded: '%s'" % len(reviews))
608608
609 def destroy(self):609 def destroy(self):
610610
=== modified file 'softwarecenter/ui/gtk3/views/appdetailsview.py'
--- softwarecenter/ui/gtk3/views/appdetailsview.py 2012-08-20 09:56:39 +0000
+++ softwarecenter/ui/gtk3/views/appdetailsview.py 2012-09-01 11:39:19 +0000
@@ -824,6 +824,14 @@
824 self.addons_to_remove = []824 self.addons_to_remove = []
825 # reviews825 # reviews
826 self.review_loader = get_review_loader(self.cache, self.db)826 self.review_loader = get_review_loader(self.cache, self.db)
827 self.review_loader.connect(
828 "get-reviews-finished", self._on_reviews_ready_callback)
829 self.review_loader.connect(
830 "remove-review", self._on_remove_review_callback)
831 self.review_loader.connect(
832 "replace-review", self._on_replace_review_callback)
833 self.review_loader.connect("update-usefulness-votes",
834 self._on_update_usefulness_votes_callback)
827835
828 # ui specific stuff836 # ui specific stuff
829 self.set_shadow_type(Gtk.ShadowType.NONE)837 self.set_shadow_type(Gtk.ShadowType.NONE)
@@ -881,6 +889,9 @@
881889
882 def _on_destroy(self, widget):890 def _on_destroy(self, widget):
883 self.cache.disconnect_by_func(self._on_cache_ready)891 self.cache.disconnect_by_func(self._on_cache_ready)
892 self.review_loader.disconnect_by_func(self._on_reviews_ready_callback)
893 self.review_loader.disconnect_by_func(self._on_remove_review_callback)
894 self.review_loader.disconnect_by_func(self._on_replace_review_callback)
884895
885 def _cache_art_assets(self):896 def _cache_art_assets(self):
886 global _asset_cache897 global _asset_cache
@@ -947,18 +958,12 @@
947 def _do_load_reviews(self):958 def _do_load_reviews(self):
948 self.reviews.show_spinner_with_message(_('Checking for reviews...'))959 self.reviews.show_spinner_with_message(_('Checking for reviews...'))
949 self.review_loader.get_reviews(960 self.review_loader.get_reviews(
950 self.app, self._reviews_ready_callback,961 self.app,
951 page=self._reviews_server_page,962 page=self._reviews_server_page,
952 language=self._reviews_server_language,963 language=self._reviews_server_language,
953 sort=self._review_sort_method,964 sort=self._review_sort_method,
954 relaxed=self._reviews_relaxed)965 relaxed=self._reviews_relaxed)
955966
956 def _review_update_single(self, action, review):
957 if action == 'replace':
958 self.reviews.replace_review(review)
959 elif action == 'remove':
960 self.reviews.remove_review(review)
961
962 def _update_review_stats_widget(self, stats):967 def _update_review_stats_widget(self, stats):
963 if stats:968 if stats:
964 # ensure that the review UI knows about the stats969 # ensure that the review UI knows about the stats
@@ -973,12 +978,23 @@
973 def _submit_reviews_done_callback(self, spawner, error):978 def _submit_reviews_done_callback(self, spawner, error):
974 self.reviews.new_review.enable()979 self.reviews.new_review.enable()
975980
976 def _reviews_ready_callback(self, app, reviews_data, my_votes=None,981 def _on_remove_review_callback(self, loader, app, review):
977 action=None, single_review=None):982 self.reviews.remove_review(review)
983 self.reviews.configure_reviews_ui()
984
985 def _on_replace_review_callback(self, loader, app, review):
986 self.reviews.replace_review(review)
987 self.reviews.configure_reviews_ui()
988
989 def _on_update_usefulness_votes_callback(self, loader, my_votes):
990 self.reviews.update_useful_votes(my_votes)
991 self.reviews.configure_reviews_ui()
992
993 def _on_reviews_ready_callback(self, loader, app, reviews_data):
978 """ callback when new reviews are ready, cleans out the994 """ callback when new reviews are ready, cleans out the
979 old ones995 old ones
980 """996 """
981 LOG.debug("_review_ready_callback: %s" % app)997 LOG.debug("_review_ready_callback: %s " % app)
982 # avoid possible race if we already moved to a new app when998 # avoid possible race if we already moved to a new app when
983 # the reviews become ready999 # the reviews become ready
984 # (we only check for pkgname currently to avoid breaking on1000 # (we only check for pkgname currently to avoid breaking on
@@ -992,7 +1008,7 @@
992 self._reviews_relaxed = True1008 self._reviews_relaxed = True
993 self._reviews_server_page = 11009 self._reviews_server_page = 1
994 self.review_loader.get_reviews(1010 self.review_loader.get_reviews(
995 self.app, self._reviews_ready_callback,1011 self.app,
996 page=self._reviews_server_page,1012 page=self._reviews_server_page,
997 language=self._reviews_server_language,1013 language=self._reviews_server_language,
998 sort=self._review_sort_method,1014 sort=self._review_sort_method,
@@ -1019,27 +1035,22 @@
1019 # update global stats cache as well1035 # update global stats cache as well
1020 self.review_loader.update_review_stats(app, stats)1036 self.review_loader.update_review_stats(app, stats)
10211037
1022 if my_votes:1038 curr_list = set(self.reviews.get_all_review_ids())
1023 self.reviews.update_useful_votes(my_votes)1039 retrieved_a_new_review = False
1040 for review in reviews_data:
1041 if not review.id in curr_list:
1042 retrieved_a_new_review = True
1043 self.reviews.add_review(review)
10241044
1025 if action:1045 if reviews_data and not retrieved_a_new_review:
1026 self._review_update_single(action, single_review)1046 # We retrieved data, but nothing new. Keep going.
1027 else:1047 self._reviews_server_page += 1
1028 curr_list = set(self.reviews.get_all_review_ids())1048 self.review_loader.get_reviews(
1029 retrieved_a_new_review = False1049 self.app,
1030 for review in reviews_data:1050 page=self._reviews_server_page,
1031 if not review.id in curr_list:1051 language=self._reviews_server_language,
1032 retrieved_a_new_review = True1052 sort=self._review_sort_method,
1033 self.reviews.add_review(review)1053 relaxed=self._reviews_relaxed)
1034 if reviews_data and not retrieved_a_new_review:
1035 # We retrieved data, but nothing new. Keep going.
1036 self._reviews_server_page += 1
1037 self.review_loader.get_reviews(
1038 self.app, self._reviews_ready_callback,
1039 page=self._reviews_server_page,
1040 language=self._reviews_server_language,
1041 sort=self._review_sort_method,
1042 relaxed=self._reviews_relaxed)
10431054
1044 self.reviews.configure_reviews_ui()1055 self.reviews.configure_reviews_ui()
10451056
@@ -1785,35 +1796,31 @@
1785 self.reviews.new_review.disable()1796 self.reviews.new_review.disable()
1786 self.review_loader.spawn_write_new_review_ui(1797 self.review_loader.spawn_write_new_review_ui(
1787 self.app, version, self.appdetails.icon, origin,1798 self.app, version, self.appdetails.icon, origin,
1788 parent_xid, self.datadir,1799 parent_xid, self.datadir)
1789 self._reviews_ready_callback,
1790 done_callback=self._submit_reviews_done_callback)
17911800
1792 def _review_report_abuse(self, review_id):1801 def _review_report_abuse(self, review_id):
1793 parent_xid = ''1802 parent_xid = ''
1794 #parent_xid = get_parent_xid(self)1803 #parent_xid = get_parent_xid(self)
1795 self.review_loader.spawn_report_abuse_ui(1804 self.review_loader.spawn_report_abuse_ui(
1796 review_id, parent_xid, self.datadir, self._reviews_ready_callback)1805 review_id, parent_xid, self.datadir)
17971806
1798 def _review_submit_usefulness(self, review_id, is_useful):1807 def _review_submit_usefulness(self, review_id, is_useful):
1799 parent_xid = ''1808 parent_xid = ''
1800 #parent_xid = get_parent_xid(self)1809 #parent_xid = get_parent_xid(self)
1801 self.review_loader.spawn_submit_usefulness_ui(1810 self.review_loader.spawn_submit_usefulness_ui(
1802 review_id, is_useful, parent_xid, self.datadir,1811 review_id, is_useful, parent_xid, self.datadir)
1803 self._reviews_ready_callback)
18041812
1805 def _review_modify(self, review_id):1813 def _review_modify(self, review_id):
1806 parent_xid = ''1814 parent_xid = ''
1807 #parent_xid = get_parent_xid(self)1815 #parent_xid = get_parent_xid(self)
1808 self.review_loader.spawn_modify_review_ui(1816 self.review_loader.spawn_modify_review_ui(
1809 parent_xid, self.appdetails.icon, self.datadir, review_id,1817 parent_xid, self.appdetails.icon, self.datadir, review_id)
1810 self._reviews_ready_callback)
18111818
1812 def _review_delete(self, review_id):1819 def _review_delete(self, review_id):
1813 parent_xid = ''1820 parent_xid = ''
1814 #parent_xid = get_parent_xid(self)1821 #parent_xid = get_parent_xid(self)
1815 self.review_loader.spawn_delete_review_ui(1822 self.review_loader.spawn_delete_review_ui(
1816 review_id, parent_xid, self.datadir, self._reviews_ready_callback)1823 review_id, parent_xid, self.datadir)
18171824
1818 # internal callbacks1825 # internal callbacks
1819 def _on_cache_ready(self, cache):1826 def _on_cache_ready(self, cache):
18201827
=== modified file 'softwarecenter/ui/qml/reviewslist.py'
--- softwarecenter/ui/qml/reviewslist.py 2012-03-07 20:08:53 +0000
+++ softwarecenter/ui/qml/reviewslist.py 2012-09-01 11:39:19 +0000
@@ -46,6 +46,11 @@
46 # FIXME: make this async46 # FIXME: make this async
47 self.cache = get_pkg_info()47 self.cache = get_pkg_info()
48 self.reviews = get_review_loader(self.cache)48 self.reviews = get_review_loader(self.cache)
49 self.reviews.connect(
50 "refresh-review-stats-finished",
51 self._on_refresh_review_stats_finished)
52 self.reviews.connect(
53 "get-reviews-finished", self._on_reviews_ready_callback)
4954
50 # QAbstractListModel code55 # QAbstractListModel code
51 def rowCount(self, parent=QModelIndex()):56 def rowCount(self, parent=QModelIndex()):
@@ -68,7 +73,7 @@
68 self._reviews = []73 self._reviews = []
69 self.endRemoveRows()74 self.endRemoveRows()
7075
71 def _on_reviews_ready_callback(self, loader, reviews):76 def _on_reviews_ready_callback(self, loader, app, reviews):
72 self.beginInsertRows(QModelIndex(),77 self.beginInsertRows(QModelIndex(),
73 self.rowCount(), # first78 self.rowCount(), # first
74 self.rowCount() + len(reviews) - 1) # last79 self.rowCount() + len(reviews) - 1) # last
@@ -87,16 +92,15 @@
8792
88 # load in the eventloop to ensure that animations are not delayed93 # load in the eventloop to ensure that animations are not delayed
89 GObject.timeout_add(94 GObject.timeout_add(
90 10, self.reviews.get_reviews, app,95 10, self.reviews.get_reviews, app, page)
91 self._on_reviews_ready_callback, page)
9296
93 # refresh review-stats (for qml)97 # refresh review-stats (for qml)
98 def _on_refresh_review_stats_finished(self, loader, stats):
99 self.reviewStatsChanged.emit()
100
94 @pyqtSlot()101 @pyqtSlot()
95 def refreshReviewStats(self):102 def refreshReviewStats(self):
96 def _on_refresh_reviews_ready_callback(loader):103 self.reviews.refresh_review_stats()
97 self.reviewStatsChanged.emit()
98 self.reviews.refresh_review_stats(
99 _on_refresh_reviews_ready_callback)
100104
101 # FIXME: how is this signal actually used in the qml JS?105 # FIXME: how is this signal actually used in the qml JS?
102 # signals106 # signals
103107
=== modified file 'tests/gtk3/test_appdetailsview.py'
--- tests/gtk3/test_appdetailsview.py 2012-08-20 09:09:45 +0000
+++ tests/gtk3/test_appdetailsview.py 2012-09-01 11:39:19 +0000
@@ -203,8 +203,8 @@
203 self.assertEqual(1, kwargs['page'])203 self.assertEqual(1, kwargs['page'])
204204
205 # Now we come back with no data205 # Now we come back with no data
206 application, callback = mock_get_reviews.call_args[0]206 application = mock_get_reviews.call_args[0][0]
207 callback(application, [])207 self.view.review_loader.emit("get-reviews-finished", application, [])
208208
209 self.assertEqual(2, mock_get_reviews.call_count)209 self.assertEqual(2, mock_get_reviews.call_count)
210 kwargs = mock_get_reviews.call_args[1]210 kwargs = mock_get_reviews.call_args[1]
@@ -237,8 +237,8 @@
237 self.assertEqual(1, kwargs['page'])237 self.assertEqual(1, kwargs['page'])
238238
239 # Now we come back with no NEW data239 # Now we come back with no NEW data
240 application, callback = mock_get_reviews.call_args[0]240 application = mock_get_reviews.call_args[0][0]
241 callback(application, reviews)241 self.view.review_loader.emit("get-reviews-finished", application, reviews)
242242
243 self.assertEqual(2, mock_get_reviews.call_count)243 self.assertEqual(2, mock_get_reviews.call_count)
244 kwargs = mock_get_reviews.call_args[1]244 kwargs = mock_get_reviews.call_args[1]

Subscribers

People subscribed via source and target branches