Merge lp:~dobey/unity-scope-click/missing-reviews into lp:unity-scope-click

Proposed by dobey
Status: Merged
Approved by: Charles Kerr
Approved revision: 414
Merged at revision: 415
Proposed branch: lp:~dobey/unity-scope-click/missing-reviews
Merge into: lp:unity-scope-click
Diff against target: 66 lines (+14/-14)
2 files modified
libclickscope/click/preview.cpp (+13/-14)
libclickscope/click/preview.h (+1/-0)
To merge this branch: bzr merge lp:~dobey/unity-scope-click/missing-reviews
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+285131@code.launchpad.net

Commit message

Refactor showing review widget to only do work if app is installed.
Add qualifier to only show review widget if app has download_url from store.
Don't check review.rating again to see if review is being submitted or not.

Description of the change

This branch should fix it so that the review entry box is always shown for installed apps in the store scope, and is not shown for side-loaded apps which are not in the store, in the apps scope. Unfortunately, this is all done inside the preview machinery, and very tightly coupled to needing the Qt bridge, so there are no new tests added in this branch, and it has to be tested manually.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
413. By dobey

Don't use the struct initializer here.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
414. By dobey

Use a member variable to cache the details instead.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libclickscope/click/preview.cpp'
--- libclickscope/click/preview.cpp 2016-01-27 16:50:50 +0000
+++ libclickscope/click/preview.cpp 2016-02-08 16:24:44 +0000
@@ -902,37 +902,36 @@
902 }902 }
903 getApplicationUri(manifest, [this, reply, manifest, app_name, &review, userid](const std::string& uri) {903 getApplicationUri(manifest, [this, reply, manifest, app_name, &review, userid](const std::string& uri) {
904 populateDetails([this, reply, uri, manifest, app_name](const PackageDetails &details){904 populateDetails([this, reply, uri, manifest, app_name](const PackageDetails &details){
905 cachedDetails = details;
905 store_department(details);906 store_department(details);
906 pushPackagePreviewWidgets(cachedWidgets, details, createButtons(uri, manifest));907 pushPackagePreviewWidgets(cachedWidgets, details, createButtons(uri, manifest));
907 },908 },
908 [this, reply, &review, manifest, userid](const ReviewList& reviewlist,909 [this, reply, &review, manifest, userid](const ReviewList& reviewlist,
909 click::Reviews::Error error) {910 click::Reviews::Error error) {
910 auto reviews = bring_to_front(reviewlist, userid);911 auto reviews = bring_to_front(reviewlist, userid);
911 scopes::PreviewWidgetList review_input;912 if (manifest.removable && !cachedDetails.download_url.empty()) {
912 bool has_reviewed = reviews.size() > 0 && reviews.front().reviewer_username == userid;913 scopes::PreviewWidgetList review_input;
914 bool has_reviewed = reviews.size() > 0 && reviews.front().reviewer_username == userid;
913915
914 if (has_reviewed) {916 if (has_reviewed) {
915 auto existing_review = reviews.front();917 auto existing_review = reviews.front();
916 reviews.pop_front();918 reviews.pop_front();
917 qDebug() << "Review for current user already exists, review id:" << existing_review.id;919 qDebug() << "Review for current user already exists, review id:" << existing_review.id;
918 if (manifest.removable) {
919 scopes::PreviewWidget rating(std::to_string(existing_review.id), "rating-edit"); // pass review id via widget id920 scopes::PreviewWidget rating(std::to_string(existing_review.id), "rating-edit"); // pass review id via widget id
920 rating.add_attribute_value("required", scopes::Variant("rating"));921 rating.add_attribute_value("required", scopes::Variant("rating"));
921 rating.add_attribute_value("review", scopes::Variant(existing_review.review_text));922 rating.add_attribute_value("review", scopes::Variant(existing_review.review_text));
922 rating.add_attribute_value("rating", scopes::Variant(existing_review.rating));923 rating.add_attribute_value("rating", scopes::Variant(existing_review.rating));
923 rating.add_attribute_value("author", scopes::Variant(existing_review.reviewer_name));924 rating.add_attribute_value("author", scopes::Variant(existing_review.reviewer_name));
924 review_input.push_back(rating);925 review_input.push_back(rating);
925 }926 } else {
926 } else {
927 if (review.rating == 0 && manifest.removable) {
928 scopes::PreviewWidget rating("rating", "rating-input");927 scopes::PreviewWidget rating("rating", "rating-input");
929 rating.add_attribute_value("required", scopes::Variant("rating"));928 rating.add_attribute_value("required", scopes::Variant("rating"));
930 review_input.push_back(rating);929 review_input.push_back(rating);
931 }930 }
931 cachedWidgets.push(review_input);
932 cachedWidgets.layout.appendToColumn(cachedWidgets.layout.singleColumn.column1, review_input);
933 cachedWidgets.layout.appendToColumn(cachedWidgets.layout.twoColumns.column1, review_input);
932 }934 }
933 cachedWidgets.push(review_input);
934 cachedWidgets.layout.appendToColumn(cachedWidgets.layout.singleColumn.column1, review_input);
935 cachedWidgets.layout.appendToColumn(cachedWidgets.layout.twoColumns.column1, review_input);
936935
937 if (error == click::Reviews::Error::NoError) {936 if (error == click::Reviews::Error::NoError) {
938 auto const revs = reviewsWidgets(reviews);937 auto const revs = reviewsWidgets(reviews);
939938
=== modified file 'libclickscope/click/preview.h'
--- libclickscope/click/preview.h 2016-01-28 12:30:31 +0000
+++ libclickscope/click/preview.h 2016-02-08 16:24:44 +0000
@@ -265,6 +265,7 @@
265private:265private:
266 scopes::ActionMetadata metadata;266 scopes::ActionMetadata metadata;
267 CachedPreviewWidgets cachedWidgets;267 CachedPreviewWidgets cachedWidgets;
268 PackageDetails cachedDetails;
268};269};
269270
270class InstalledScopePreview : public PreviewStrategy271class InstalledScopePreview : public PreviewStrategy

Subscribers

People subscribed via source and target branches

to all changes: