Merge lp:~dobey/unity-scope-click/full-langs into lp:unity-scope-click/devel

Proposed by dobey
Status: Merged
Approved by: dobey
Approved revision: 269
Merged at revision: 269
Proposed branch: lp:~dobey/unity-scope-click/full-langs
Merge into: lp:unity-scope-click/devel
Diff against target: 140 lines (+66/-4)
5 files modified
libclickscope/click/configuration.cpp (+15/-0)
libclickscope/click/configuration.h (+2/-0)
libclickscope/tests/test_configuration.cpp (+12/-0)
scope/click/reviews.cpp (+9/-2)
scope/tests/test_reviews.cpp (+28/-2)
To merge this branch: bzr merge lp:~dobey/unity-scope-click/full-langs
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alejandro J. Cura (community) Approve
Paweł Stołowski (community) Needs Fixing
Review via email: mp+220523@code.launchpad.net

Commit message

Handle some languages differently when submitting reviews.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote :

Looks good, just one minor nitpick:

26 + if (std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)
27 + != FULL_LANG_CODES.end()) {
28 + return true;
29 + }
30 + return false;

can you reduce it to just:
return std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)!= FULL_LANG_CODES.end();

review: Needs Fixing
269. By dobey

Clean is_full_lang_code function per review.

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Branch looks good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libclickscope/click/configuration.cpp'
--- libclickscope/click/configuration.cpp 2014-05-16 21:44:07 +0000
+++ libclickscope/click/configuration.cpp 2014-05-22 16:34:27 +0000
@@ -40,6 +40,15 @@
4040
41namespace click {41namespace click {
4242
43/* NOTE: The list of languages we need to use the full language code for.
44 * Please keep this list in a-z order.
45 */
46const std::vector<const char*> Configuration::FULL_LANG_CODES = {
47 "pt_BR",
48 "zh_CN",
49 "zh_TW",
50};
51
43std::vector<std::string> Configuration::list_folder(const std::string& folder, const std::string& pattern)52std::vector<std::string> Configuration::list_folder(const std::string& folder, const std::string& pattern)
44{53{
45 std::vector<std::string> result;54 std::vector<std::string> result;
@@ -121,4 +130,10 @@
121 return result;130 return result;
122}131}
123132
133bool Configuration::is_full_lang_code(const std::string& language)
134{
135 return std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)
136 != FULL_LANG_CODES.end();
137}
138
124} // namespace click139} // namespace click
125140
=== modified file 'libclickscope/click/configuration.h'
--- libclickscope/click/configuration.h 2014-05-16 21:44:07 +0000
+++ libclickscope/click/configuration.h 2014-05-22 16:34:27 +0000
@@ -43,6 +43,7 @@
43 constexpr static const char* FRAMEWORKS_PATTERN {"*.framework"};43 constexpr static const char* FRAMEWORKS_PATTERN {"*.framework"};
44 constexpr static const int FRAMEWORKS_EXTENSION_LENGTH = 10; // strlen(".framework")44 constexpr static const int FRAMEWORKS_EXTENSION_LENGTH = 10; // strlen(".framework")
45 constexpr static const char* LANGUAGE_ENVVAR {"LANGUAGE"};45 constexpr static const char* LANGUAGE_ENVVAR {"LANGUAGE"};
46 static const std::vector<const char*> FULL_LANG_CODES;
4647
47 virtual std::vector<std::string> get_available_frameworks();48 virtual std::vector<std::string> get_available_frameworks();
48 virtual std::string get_architecture();49 virtual std::string get_architecture();
@@ -50,6 +51,7 @@
50 virtual std::string get_language_base();51 virtual std::string get_language_base();
51 virtual std::string get_language();52 virtual std::string get_language();
52 virtual std::string get_accept_languages();53 virtual std::string get_accept_languages();
54 static bool is_full_lang_code(const std::string& language);
5355
54 virtual ~Configuration() {}56 virtual ~Configuration() {}
55protected:57protected:
5658
=== modified file 'libclickscope/tests/test_configuration.cpp'
--- libclickscope/tests/test_configuration.cpp 2014-05-16 21:44:07 +0000
+++ libclickscope/tests/test_configuration.cpp 2014-05-22 16:34:27 +0000
@@ -172,3 +172,15 @@
172 EXPECT_EQ(Configuration().get_accept_languages(), "en");172 EXPECT_EQ(Configuration().get_accept_languages(), "en");
173 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);173 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
174}174}
175
176TEST(Configuration, isFullLangCodeTestAllFullLangCodes)
177{
178 for (auto lang: Configuration::FULL_LANG_CODES) {
179 EXPECT_TRUE(Configuration::is_full_lang_code(lang));
180 }
181}
182
183TEST(Configuration, isFullLangCodeReturnsFalseForEN)
184{
185 ASSERT_FALSE(Configuration::is_full_lang_code("en_US"));
186}
175187
=== modified file 'scope/click/reviews.cpp'
--- scope/click/reviews.cpp 2014-05-07 21:22:59 +0000
+++ scope/click/reviews.cpp 2014-05-22 16:34:27 +0000
@@ -139,8 +139,15 @@
139 root["review_text"] = review.review_text;139 root["review_text"] = review.review_text;
140140
141 root["arch_tag"] = click::Configuration().get_architecture();141 root["arch_tag"] = click::Configuration().get_architecture();
142 // NOTE: We only use the base language code for reviews.142 /* NOTE: We only use the base language code for reviews, except for
143 root["language"] = click::Configuration().get_language_base();143 * codes in the click::Configuration::FULL_LANG_CODES vector.
144 */
145 auto language = click::Configuration().get_language();
146 if (click::Configuration::is_full_lang_code(language)) {
147 root["language"] = language;
148 } else {
149 root["language"] = click::Configuration().get_language_base();
150 }
144151
145 // NOTE: "summary" is a required field, but we don't have one. Use "".152 // NOTE: "summary" is a required field, but we don't have one. Use "".
146 root["summary"] = "Review";153 root["summary"] = "Review";
147154
=== modified file 'scope/tests/test_reviews.cpp'
--- scope/tests/test_reviews.cpp 2014-05-20 19:33:41 +0000
+++ scope/tests/test_reviews.cpp 2014-05-22 16:34:27 +0000
@@ -275,8 +275,8 @@
275 review.package_version = "0.1";275 review.package_version = "0.1";
276276
277 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,277 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
278 "zh_TW.UTF-8", 1), 0);278 "es_AR.UTF-8", 1), 0);
279 std::string expected_language = "\"language\":\"zh\"";279 std::string expected_language = "\"language\":\"es\"";
280 EXPECT_CALL(*clientPtr, callImpl(_, "POST", true, _,280 EXPECT_CALL(*clientPtr, callImpl(_, "POST", true, _,
281 HasSubstr(expected_language), _))281 HasSubstr(expected_language), _))
282 .Times(1)282 .Times(1)
@@ -287,6 +287,32 @@
287 ASSERT_EQ(unsetenv(click::Configuration::LANGUAGE_ENVVAR), 0);287 ASSERT_EQ(unsetenv(click::Configuration::LANGUAGE_ENVVAR), 0);
288}288}
289289
290TEST_F(ReviewsTest, testSubmitReviewLanguageCorrectForFullLangCodes)
291{
292 LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
293 auto response = responseForReply(reply.asSharedPtr());
294
295 click::Review review;
296 review.rating = 3;
297 review.review_text = "A review.";
298 review.package_name = "com.example.test";
299 review.package_version = "0.1";
300
301 for (std::string lang: click::Configuration::FULL_LANG_CODES) {
302 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
303 lang.c_str(), 1), 0);
304 std::string expected_language = "\"language\":\"" + lang + "\"";
305 EXPECT_CALL(*clientPtr, callImpl(_, "POST", true, _,
306 HasSubstr(expected_language), _))
307 .Times(1)
308 .WillOnce(Return(response));
309
310 auto submit_op = reviewsPtr->submit_review(review,
311 [](click::Reviews::Error){});
312 ASSERT_EQ(unsetenv(click::Configuration::LANGUAGE_ENVVAR), 0);
313 }
314}
315
290TEST_F(ReviewsTest, testGetBaseUrl)316TEST_F(ReviewsTest, testGetBaseUrl)
291{317{
292 const char *value = getenv(click::REVIEWS_BASE_URL_ENVVAR.c_str());318 const char *value = getenv(click::REVIEWS_BASE_URL_ENVVAR.c_str());

Subscribers

People subscribed via source and target branches

to all changes: