Merge lp:~aaronp/software-center/fix-794060-for-4.0 into lp:software-center/4.0

Proposed by Aaron Peachey
Status: Merged
Merged at revision: 1774
Proposed branch: lp:~aaronp/software-center/fix-794060-for-4.0
Merge into: lp:software-center/4.0
Diff against target: 94 lines (+34/-6)
3 files modified
softwarecenter/db/reviews.py (+3/-3)
softwarecenter/view/appdetailsview_gtk.py (+15/-3)
softwarecenter/view/widgets/reviews.py (+16/-0)
To merge this branch: bzr merge lp:~aaronp/software-center/fix-794060-for-4.0
Reviewer Review Type Date Requested Status
software-store-developers Pending
Review via email: mp+64183@code.launchpad.net

Description of the change

Fixes lp:#794060 which was resulting in review duplication after a review activity (e.g. submit usefulness) as pagination code introduced in 4.0.3 no longer clears the reviews in the app details view before reloading them.

Once (and if) this branch is approved, I'll re-write the fix separately for trunk too (since the refactoring has changed alot of the structure in trunk since 4.0).

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

On Fri, Jun 10, 2011 at 01:38:36PM -0000, Aaron Peachey wrote:
> Aaron Peachey has proposed merging lp:~aaronp/software-center/fix-794060-for-4.0 into lp:software-center/4.0.
>
> Requested reviews:
> software-store-developers (software-store-developers)
> Related bugs:
> Bug #794060 in software-center (Ubuntu): "Software Centre duplicate reviews"
> https://bugs.launchpad.net/ubuntu/+source/software-center/+bug/794060
>
> For more details, see:
> https://code.launchpad.net/~aaronp/software-center/fix-794060-for-4.0/+merge/64183
>
> Fixes lp:#794060 which was resulting in review duplication after a review activity (e.g. submit usefulness) as pagination code introduced in 4.0.3 no longer clears the reviews in the app details view before reloading them.
>
> Once (and if) this branch is approved, I'll re-write the fix separately for trunk too (since the refactoring has changed alot of the structure in trunk since 4.0).
[..]

Indeed, thanks a lot for this! This is a good catch. I merged it now
and will push it to proposed next.

Thanks,
 Michael

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'softwarecenter/db/reviews.py'
--- softwarecenter/db/reviews.py 2011-05-10 13:35:36 +0000
+++ softwarecenter/db/reviews.py 2011-06-10 13:38:30 +0000
@@ -299,7 +299,7 @@
299 if str(review.id) == str(review_id):299 if str(review.id) == str(review_id):
300 # remove the one we don't want to see anymore300 # remove the one we don't want to see anymore
301 self._reviews[app].remove(review)301 self._reviews[app].remove(review)
302 callback(app, self._reviews[app])302 callback(app, self._reviews[app], None, 'remove', review)
303 break303 break
304304
305 def _on_submit_usefulness_finished(self, pid, status, (review_id, is_useful, stdout_fd, callback)):305 def _on_submit_usefulness_finished(self, pid, status, (review_id, is_useful, stdout_fd, callback)):
@@ -322,7 +322,7 @@
322 review.usefulness_total = getattr(review, "usefulness_total", 0) + 1322 review.usefulness_total = getattr(review, "usefulness_total", 0) + 1
323 if is_useful:323 if is_useful:
324 review.usefulness_favorable = getattr(review, "usefulness_favorable", 0) + 1324 review.usefulness_favorable = getattr(review, "usefulness_favorable", 0) + 1
325 callback(app, self._reviews[app], useful_votes)325 callback(app, self._reviews[app], useful_votes, 'replace', review)
326 break326 break
327 else:327 else:
328 LOG.debug("submit usefulness id=%s failed with exitcode %s" % (328 LOG.debug("submit usefulness id=%s failed with exitcode %s" % (
@@ -331,7 +331,7 @@
331 for review in reviews:331 for review in reviews:
332 if str(review.id) == str(review_id):332 if str(review.id) == str(review_id):
333 review.usefulness_submit_error = exitcode333 review.usefulness_submit_error = exitcode
334 callback(app, self._reviews[app])334 callback(app, self._reviews[app], None, 'replace', review)
335 break335 break
336336
337337
338338
=== modified file 'softwarecenter/view/appdetailsview_gtk.py'
--- softwarecenter/view/appdetailsview_gtk.py 2011-05-10 14:40:05 +0000
+++ softwarecenter/view/appdetailsview_gtk.py 2011-06-10 13:38:30 +0000
@@ -785,8 +785,17 @@
785 self.review_loader.get_reviews(785 self.review_loader.get_reviews(
786 self.app, self._reviews_ready_callback,786 self.app, self._reviews_ready_callback,
787 page=self._reviews_server_page)787 page=self._reviews_server_page)
788
789 def _review_update_single(self, action, review):
790 if action == 'replace':
791 self.reviews.replace_review(review)
792 elif action == 'remove':
793 self.reviews.remove_review(review)
794 return
795
788796
789 def _reviews_ready_callback(self, app, reviews_data, my_votes=None):797 def _reviews_ready_callback(self, app, reviews_data, my_votes=None,
798 action=None, single_review=None):
790 """ callback when new reviews are ready, cleans out the799 """ callback when new reviews are ready, cleans out the
791 old ones800 old ones
792 """801 """
@@ -819,8 +828,11 @@
819 if my_votes:828 if my_votes:
820 self.reviews.update_useful_votes(my_votes)829 self.reviews.update_useful_votes(my_votes)
821 830
822 for review in reviews_data:831 if action:
823 self.reviews.add_review(review)832 self._review_update_single(action, single_review)
833 else:
834 for review in reviews_data:
835 self.reviews.add_review(review)
824 self.reviews.configure_reviews_ui()836 self.reviews.configure_reviews_ui()
825837
826 def on_test_drive_clicked(self, button):838 def on_test_drive_clicked(self, button):
827839
=== modified file 'softwarecenter/view/widgets/reviews.py'
--- softwarecenter/view/widgets/reviews.py 2011-05-31 06:23:01 +0000
+++ softwarecenter/view/widgets/reviews.py 2011-06-10 13:38:30 +0000
@@ -881,6 +881,22 @@
881 def add_review(self, review):881 def add_review(self, review):
882 self.reviews.append(review)882 self.reviews.append(review)
883 return883 return
884
885 def replace_review(self, review):
886 for r in self.reviews:
887 if r.id == review.id:
888 pos = self.reviews.index(r)
889 self.reviews.remove(r)
890 self.reviews.insert(pos, review)
891 break
892 return
893
894 def remove_review(self, review):
895 for r in self.reviews:
896 if r.id == review.id:
897 self.reviews.remove(r)
898 break
899 return
884900
885 def clear(self):901 def clear(self):
886 self.reviews = []902 self.reviews = []

Subscribers

People subscribed via source and target branches