Merge lp:~paulliu/unity-scope-click/submit-reviews into lp:unity-scope-click

Proposed by Ying-Chun Liu
Status: Rejected
Rejected by: dobey
Proposed branch: lp:~paulliu/unity-scope-click/submit-reviews
Merge into: lp:unity-scope-click
Diff against target: 178 lines (+132/-0)
2 files modified
src/click-scope.vala (+9/-0)
src/rnrclient.vala (+123/-0)
To merge this branch: bzr merge lp:~paulliu/unity-scope-click/submit-reviews
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
dobey (community) Needs Fixing
Review via email: mp+201584@code.launchpad.net

Commit message

Add submit reviews action in scope.

Description of the change

submit reviews.

To post a comment you must log in.
108. By Ying-Chun Liu on 2014-01-14

fix indent.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
109. By Ying-Chun Liu on 2014-01-14

remove appname.

110. By Ying-Chun Liu on 2014-01-14

use details.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
111. By Ying-Chun Liu on 2014-01-14

submit architecture.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
dobey (dobey) wrote :

While the basic premise of this branch is fine, I don't think there is any way to actually type in a review in the preview. I think we'll need work in the dash, and possibly a different preview to actually write the review in.

84 + debug ("postargs = "+postargs);
85 + debug ("surl = "+surl);
113 + debug("oAuthValue = "+oauthValue);
118 + debug("message data = "+((string) message.request_body.data));
107 + debug("JSON = "+json);

Let's get rid of these two debug lines. They are very vague, and we shouldn't be logging the signature anyway. Also, we don't need/want to log the entire JSON or message body. For the rest of the debug messages you're adding, please use more descriptive strings than ("errmsg = " + errmsg). If the message itself has enough information, it may be acceptable to just debug(message). In other cases, add descriptive context to the messages, such as the function where the debug() call is being made. Please do not abbreviate things in log messages either, such as using "err" instead of "error" in some places.

+ oauthValue = oauthValue + ", "+key+"=\""+Soup.URI.encode(val,"=/&")+"\"";

Use space characters between operators and values, please. Also, after commas in argument lists passed to functions, and between lvalue and rvalue in new variable initialization. There are several inconsistencies in the style in the code that makes it a bit difficult to read in places, due to lack of white space.

review: Needs Fixing
112. By Ying-Chun Liu on 2014-01-17

Always add review.

113. By Ying-Chun Liu on 2014-01-17

fix indent. remove some redudent debug()

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Ying-Chun Liu (paulliu) wrote :

Let's stop this. Seems to me that we currently don't have a way to send the review_text back to the scope.
There's execute(actionID, {hashTable}) in unity8, but that hashTable isn't exposed in the scope API.

Ying-Chun Liu (paulliu) wrote :

BTW, the corresponding unity8 branch is lp:~paulliu/unity8/rnr

dobey (dobey) wrote :

Rejecting this since we can't actually get any reviews from the dash to submit to the server, and we're rewriting to use the C++ APIs now anyway.

Let's get the dash fixed when it switches to the new API as well, so we can actually implement submission of reviews.

Unmerged revisions

113. By Ying-Chun Liu on 2014-01-17

fix indent. remove some redudent debug()

112. By Ying-Chun Liu on 2014-01-17

Always add review.

111. By Ying-Chun Liu on 2014-01-14

submit architecture.

110. By Ying-Chun Liu on 2014-01-14

use details.

109. By Ying-Chun Liu on 2014-01-14

remove appname.

108. By Ying-Chun Liu on 2014-01-14

fix indent.

107. By Ying-Chun Liu on 2014-01-14

Merge upstream

106. By Ying-Chun Liu on 2014-01-14

Add submit

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/click-scope.vala'
2--- src/click-scope.vala 2014-01-13 22:24:22 +0000
3+++ src/click-scope.vala 2014-01-17 14:26:40 +0000
4@@ -26,6 +26,7 @@
5 private const string ACTION_CONFIRM_UNINSTALL = "confirm_uninstall";
6 private const string ACTION_CLOSE_PREVIEW = "close_preview";
7 private const string ACTION_OPEN_ACCOUNTS = "open_accounts";
8+private const string ACTION_SUBMIT_REVIEW = "submit_review";
9
10 private const string ACCOUNT_SETTINGS_URL = "settings:///system/online-accounts";
11
12@@ -133,6 +134,7 @@
13 preview.add_info(new Unity.InfoHint.with_variant(HINT_REVIEWS, LABEL_REVIEWS, null, reviews));
14 debug("Got %d reviews for %s", (int)reviews.n_children(), details.package_name);
15 }
16+ preview.add_action (new Unity.PreviewAction (ACTION_SUBMIT_REVIEW, ("*** submit_review"), null));
17 return preview;
18 } catch (WebserviceError e) {
19 debug ("Error calling webservice: %s", e.message);
20@@ -274,6 +276,13 @@
21 } else if (action_id == ACTION_OPEN_ACCOUNTS) {
22 return new Unity.ActivationResponse (Unity.HandledType.NOT_HANDLED,
23 ACCOUNT_SETTINGS_URL);
24+ } else if (action_id == ACTION_SUBMIT_REVIEW) {
25+ var details = yield webservice.get_details(app_id);
26+ var summary = result.metadata.get("summary").get_string();
27+ var review_text = result.metadata.get("review_text").get_string();
28+ var rating = result.metadata.get("rating").get_string();
29+ yield rnrClient.submit_review(details, summary, review_text, rating);
30+ return null;
31 } else {
32 return null;
33 }
34
35=== modified file 'src/rnrclient.vala'
36--- src/rnrclient.vala 2014-01-09 13:53:51 +0000
37+++ src/rnrclient.vala 2014-01-17 14:26:40 +0000
38@@ -55,6 +55,10 @@
39 return get_base_url() + "/api/1.0/reviews/filter/%s/%s/%s/%s/%s/";
40 }
41
42+ private string get_submit_review_url() {
43+ return get_base_url() + "/api/1.0/reviews/";
44+ }
45+
46 /**
47 * get_reviews:
48 * @details: the AppDetails describe the details.
49@@ -171,6 +175,98 @@
50 ret = builder.end();
51 return ret;
52 }
53+
54+ public async void submit_review(AppDetails details, string summary, string review_text, string rating) {
55+ var rc = new ReviewContent();
56+ rc.packagename = details.package_name;
57+ rc.origin = rc.packagename;
58+ rc.summary = summary;
59+ rc.review_text = review_text;
60+ rc.rating = rating;
61+ rc.distroseries = "click";
62+ rc.version = details.version;
63+ rc.arch_tag = ClickInterface.get_arch();
64+
65+ string[] languages = GLib.Intl.get_language_names();
66+ if (languages.length > 0) {
67+ rc.language = languages[0];
68+ }
69+
70+ yield submit_review_by_review_content(rc);
71+ }
72+
73+ public async void submit_review_by_review_content(ReviewContent rc) {
74+ var credentials = yield u1creds.get_credentials();
75+ var url = get_submit_review_url();
76+ string postargs;
77+ var surl = OAuth.sign_url2(url, out postargs,
78+ OAuth.Method.PLAINTEXT, "POST",
79+ credentials[CONSUMER_KEY],
80+ credentials[CONSUMER_SECRET],
81+ credentials[TOKEN],
82+ credentials[TOKEN_SECRET]);
83+
84+ var message = new Soup.Message ("POST", surl);
85+ var gen = new Json.Generator();
86+ var root = new Json.Node(NodeType.OBJECT);
87+ var object = new Json.Object();
88+ root.set_object(object);
89+ gen.set_root(root);
90+
91+ var args = new Json.Object();
92+ object.set_string_member("package_name", rc.packagename);
93+ object.set_string_member("summary", rc.summary);
94+ object.set_string_member("version", rc.version);
95+ object.set_string_member("review_text", rc.review_text);
96+ object.set_string_member("distroseries", rc.distroseries);
97+ object.set_string_member("arch_tag", rc.arch_tag);
98+ object.set_string_member("rating", rc.rating);
99+ object.set_string_member("language", rc.language);
100+ object.set_string_member("origin", rc.origin);
101+
102+ size_t length;
103+ string json = gen.to_data(out length);
104+ HashTable<string,string> oauthTable = Soup.Form.decode(postargs);
105+ string oauthValue = "OAuth realm=\"Ratings%20and%20Reviews\"";
106+ oauthTable.foreach( (key, val) => {
107+ oauthValue = oauthValue + ", " + key + "=\"" + Soup.URI.encode(val,"=/&") + "\"";
108+ });
109+ message.request_headers.set_content_type("application/json", null);
110+ message.request_headers.append("Authorization", oauthValue);
111+ message.request_body.append(Soup.MemoryUse.COPY, json.data);
112+ message.request_body.flatten();
113+
114+ string response="";
115+ WebserviceError failure = null;
116+
117+ http_session.queue_message(message, (http_session, message) => {
118+ if (message.status_code != Soup.KnownStatusCode.OK) {
119+ var errmsg = "Web request failed: HTTP %u %s".printf(
120+ message.status_code, message.reason_phrase);
121+ var errmsg_len = message.response_headers.get_content_length();
122+ debug ("error content length = " + errmsg_len.to_string());
123+ message.response_headers.foreach((name, val) => {
124+ debug ("error header %s:%s", name, val);
125+ });
126+
127+ failure = new WebserviceError.HTTP_ERROR(errmsg);
128+ debug (errmsg);
129+ } else {
130+ message.response_body.flatten ();
131+ response = (string) message.response_body.data;
132+ debug ("response is %s", response);
133+ }
134+ submit_review_by_review_content.callback();
135+ });
136+ yield;
137+ if (failure != null) {
138+ debug("cannot submit reviews error %s", failure.message);
139+ }
140+
141+ return;
142+ }
143+
144+
145 }
146
147 /**
148@@ -199,3 +295,30 @@
149 public string distroseries = "any";
150 public string version = "any";
151 }
152+
153+/**
154+ * ReviewContent:
155+ * @packagename: the name of package.
156+ * @language: the language string. Eg: en zh ...
157+ * @origin: Normally comes from "ubuntu". For click packages it is "click"
158+ * @distroseries: For example, "natty", "saucy".
159+ * @version: The version of the software.
160+ * @summary: the summary of a review.
161+ * @review_text: the review text.
162+ * @rating: the ratings of the review.
163+ * @arch_tag: the architecture tag. Eg: i386 armhf ...
164+ *
165+ * ReviewContent is the class used to submit the reviews to the server.
166+ *
167+ */
168+public class ReviewContent : GLib.Object {
169+ public string packagename = "";
170+ public string language = "";
171+ public string origin = "";
172+ public string distroseries = "";
173+ public string version = "";
174+ public string summary = "";
175+ public string review_text = "";
176+ public string rating = "";
177+ public string arch_tag = "";
178+}

Subscribers

People subscribed via source and target branches

to all changes: