Commit 1969bdf6 authored by treib's avatar treib Committed by Commit bot

[NTP Snippets] Overwrite the title of the ARTICLES if provided by the server

BUG=653808

Review-Url: https://codereview.chromium.org/2393353005
Cr-Commit-Position: refs/heads/master@{#423853}
parent 1017ec5e
...@@ -520,10 +520,14 @@ void NTPSnippetsService::OnFetchFinished( ...@@ -520,10 +520,14 @@ void NTPSnippetsService::OnFetchFinished(
*fetched_categories) { *fetched_categories) {
Category category = fetched_category.category; Category category = fetched_category.category;
// The ChromeReader backend doesn't provide category titles, so don't
// overwrite the existing title for ARTICLES if the new one is empty.
// TODO(treib): Remove this check after we fully switch to the content
// suggestions backend.
// TODO(sfiera): Avoid hard-coding articles category checks in so many // TODO(sfiera): Avoid hard-coding articles category checks in so many
// places. // places.
if (category != articles_category_) { if (category != articles_category_ ||
// Only update titles from server-side provided categories. !fetched_category.localized_title.empty()) {
categories_[category].localized_title = categories_[category].localized_title =
fetched_category.localized_title; fetched_category.localized_title;
} }
......
...@@ -90,6 +90,8 @@ const char kSnippetAmpUrl[] = "http://localhost/amp"; ...@@ -90,6 +90,8 @@ const char kSnippetAmpUrl[] = "http://localhost/amp";
const char kSnippetUrl2[] = "http://foo.com/bar"; const char kSnippetUrl2[] = "http://foo.com/bar";
const char kTestJsonDefaultCategoryTitle[] = "Some title";
base::Time GetDefaultCreationTime() { base::Time GetDefaultCreationTime() {
base::Time out_time; base::Time out_time;
EXPECT_TRUE(base::Time::FromUTCExploded(kDefaultCreationTime, &out_time)); EXPECT_TRUE(base::Time::FromUTCExploded(kDefaultCreationTime, &out_time));
...@@ -100,18 +102,28 @@ base::Time GetDefaultExpirationTime() { ...@@ -100,18 +102,28 @@ base::Time GetDefaultExpirationTime() {
return base::Time::Now() + base::TimeDelta::FromHours(1); return base::Time::Now() + base::TimeDelta::FromHours(1);
} }
std::string GetTestJson(const std::vector<std::string>& snippets) { std::string GetTestJson(const std::vector<std::string>& snippets,
const std::string& category_title) {
return base::StringPrintf( return base::StringPrintf(
"{\n" "{\n"
" \"categories\": [{\n" " \"categories\": [{\n"
" \"id\": 1,\n" " \"id\": 1,\n"
" \"localizedTitle\": \"Articles for You\",\n" " \"localizedTitle\": \"%s\",\n"
" \"suggestions\": [%s]\n" " \"suggestions\": [%s]\n"
" }]\n" " }]\n"
"}\n", "}\n",
category_title.c_str(),
base::JoinString(snippets, ", ").c_str()); base::JoinString(snippets, ", ").c_str());
} }
std::string GetTestJson(const std::vector<std::string>& snippets) {
return GetTestJson(snippets, kTestJsonDefaultCategoryTitle);
}
std::string GetTestJsonWithoutTitle(const std::vector<std::string>& snippets) {
return GetTestJson(snippets, std::string());
}
std::string GetMultiCategoryJson(const std::vector<std::string>& articles, std::string GetMultiCategoryJson(const std::vector<std::string>& articles,
const std::vector<std::string>& others) { const std::vector<std::string>& others) {
return base::StringPrintf( return base::StringPrintf(
...@@ -437,7 +449,7 @@ class NTPSnippetsServiceTest : public ::testing::Test { ...@@ -437,7 +449,7 @@ class NTPSnippetsServiceTest : public ::testing::Test {
// Add an initial fetch response, as the service tries to fetch when there // Add an initial fetch response, as the service tries to fetch when there
// is nothing in the DB. // is nothing in the DB.
if (set_empty_response) if (set_empty_response)
SetUpFetchResponse(GetTestJson(std::vector<std::string>())); SetUpFetchResponse(GetTestJsonWithoutTitle(std::vector<std::string>()));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
observer_->WaitForLoad(); observer_->WaitForLoad();
...@@ -642,6 +654,43 @@ TEST_F(NTPSnippetsServiceTest, Full) { ...@@ -642,6 +654,43 @@ TEST_F(NTPSnippetsServiceTest, Full) {
EXPECT_EQ(GURL(kSnippetAmpUrl), suggestion.amp_url()); EXPECT_EQ(GURL(kSnippetAmpUrl), suggestion.amp_url());
} }
TEST_F(NTPSnippetsServiceTest, CategoryTitle) {
const base::string16 response_title =
base::UTF8ToUTF16(kTestJsonDefaultCategoryTitle);
auto service = MakeSnippetsService();
// The articles category should be there by default, and have a title.
CategoryInfo info_before = service->GetCategoryInfo(articles_category());
ASSERT_FALSE(info_before.title().empty());
ASSERT_NE(info_before.title(), response_title);
std::string json_str_no_title(GetTestJsonWithoutTitle({GetSnippet()}));
LoadFromJSONString(service.get(), json_str_no_title);
ASSERT_THAT(observer().SuggestionsForCategory(articles_category()),
SizeIs(1));
ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
// The response didn't contain a category title. Make sure we didn't touch
// the existing one.
CategoryInfo info_no_title = service->GetCategoryInfo(articles_category());
EXPECT_EQ(info_before.title(), info_no_title.title());
std::string json_str_with_title(GetTestJson({GetSnippet()}));
LoadFromJSONString(service.get(), json_str_with_title);
ASSERT_THAT(observer().SuggestionsForCategory(articles_category()),
SizeIs(1));
ASSERT_THAT(service->GetSnippetsForTesting(articles_category()), SizeIs(1));
// This time, the response contained a title, |kTestJsonDefaultCategoryTitle|.
// Make sure we updated the title in the CategoryInfo.
CategoryInfo info_with_title = service->GetCategoryInfo(articles_category());
EXPECT_NE(info_before.title(), info_with_title.title());
EXPECT_EQ(response_title, info_with_title.title());
}
TEST_F(NTPSnippetsServiceTest, MultipleCategories) { TEST_F(NTPSnippetsServiceTest, MultipleCategories) {
std::string json_str( std::string json_str(
GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)})); GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)}));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment