Skip to content

Commit c08eccf

Browse files
kishansinghifs1pritesh299Mohak51234mosin74
authored
Fix oppia#23323 : Fix blog pagination for deleted authors (oppia#24732)
* Fix blog author details handling and improve pagination for deleted authors * Fix blog author details handling by removing unnecessary model creation in tests * Add blog author details model creation in dashboard tests * Fix blog author details handling and ensure author model creation during publishing * Remove unused blog_models import and related mypy handling in blog_dashboard.py * Remove unnecessary author ID initialization and add assertions in blog dashboard tests * Fix blog dashboard test to handle missing author model scenario * Fix oppia#24233: Stabilize donate page acceptance test by waiting for DonorBox iframe (oppia#24675) fix the issue * Fix oppia#24204: Prevent text overflow and layout break in classroom page (oppia#24608) * fix: prevent text overflow and improve wrapping in classroom page * fix: prevent text overflow in learn navigation popup * Update blog post editor tests for blogPostWriter role - Update usernames in blog post editor and writer tests - Fix formatting in blog_dashboard.py - Set dummy author and user bio correctly in tests * Fix blog author details retrieval to return default values when missing * Fix blog editor test to include user profile setup before publishing --------- Co-authored-by: Pritesh Jogdhankar <129420569+pritesh299@users.noreply.github.com> Co-authored-by: Mohak51234 <143473709+Mohak51234@users.noreply.github.com> Co-authored-by: Mosin Shaikh <123667613+mosin74@users.noreply.github.com>
1 parent 2cadd42 commit c08eccf

File tree

10 files changed

+258
-50
lines changed

10 files changed

+258
-50
lines changed

‎core/controllers/blog_dashboard.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030

3131
from typing import Dict, List, Optional, TypedDict
3232

33+
MYPY = False
34+
if MYPY: # pragma: no cover
35+
pass
36+
3337

3438
class BlogCardSummaryDict(TypedDict):
3539
"""Type for the dict representation of blog_card_summary_dict."""
@@ -114,9 +118,10 @@ class BlogDashboardDataHandler(
114118
def get(self) -> None:
115119
"""Retrieves data for the blog dashboard."""
116120
assert self.user_id is not None
117-
author_details = blog_services.get_blog_author_details(
118-
self.user_id
121+
author_details_dict = blog_services.get_blog_author_details(
122+
self.user_id, strict=False
119123
).to_dict()
124+
120125
no_of_published_blog_posts = 0
121126
published_post_summary_dicts = []
122127
no_of_draft_blog_posts = 0
@@ -148,7 +153,7 @@ def get(self) -> None:
148153
)
149154
self.values.update(
150155
{
151-
'author_details': author_details,
156+
'author_details': author_details_dict,
152157
'no_of_published_blog_posts': no_of_published_blog_posts,
153158
'no_of_draft_blog_posts': no_of_draft_blog_posts,
154159
'published_blog_post_summary_dicts': published_post_summary_dicts,
@@ -175,13 +180,13 @@ def put(self) -> None:
175180
blog_services.update_blog_author_details(
176181
self.user_id, displayed_author_name, author_bio
177182
)
178-
author_details = blog_services.get_blog_author_details(
183+
author_details_dict = blog_services.get_blog_author_details(
179184
self.user_id
180185
).to_dict()
181186

182187
self.values.update(
183188
{
184-
'author_details': author_details,
189+
'author_details': author_details_dict,
185190
}
186191
)
187192
self.render_json(self.values)

‎core/controllers/blog_dashboard_test.py‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,12 @@
2626

2727
MYPY = False
2828
if MYPY: # pragma: no cover
29-
from mypy_imports import user_models
29+
from mypy_imports import blog_models, user_models
3030

31-
(user_models,) = models.Registry.import_models([models.Names.USER])
31+
(
32+
blog_models,
33+
user_models,
34+
) = models.Registry.import_models([models.Names.BLOG, models.Names.USER])
3235

3336

3437
class BlogDashboardDataHandlerTests(test_utils.GenericTestBase):
@@ -51,6 +54,9 @@ def setUp(self) -> None:
5154
self.BLOG_EDITOR_EMAIL
5255
)
5356

57+
blog_services.create_blog_author_details_model(self.blog_admin_id)
58+
blog_services.create_blog_author_details_model(self.blog_editor_id)
59+
5460
def test_get_dashboard_page_data(self) -> None:
5561
# Checks blog editor can access blog dashboard.
5662
self.login(self.BLOG_EDITOR_EMAIL)
@@ -112,6 +118,42 @@ def test_get_dashboard_page_data(self) -> None:
112118
)
113119
self.assertEqual(json_response['draft_blog_post_summary_dicts'], [])
114120

121+
def test_get_dashboard_with_missing_author_model(self) -> None:
122+
"""Tests that dashboard returns default author details when
123+
the author model is missing.
124+
"""
125+
self.signup('neweditor@example.com', 'NewEditor')
126+
new_editor_id = self.get_user_id_from_email('neweditor@example.com')
127+
self.add_user_role('NewEditor', feconf.ROLE_ID_BLOG_POST_EDITOR)
128+
self.login('neweditor@example.com')
129+
# Create a blog post to trigger author model creation.
130+
csrf_token = self.get_new_csrf_token()
131+
self.post_json(
132+
'%s' % feconf.BLOG_DASHBOARD_DATA_URL,
133+
{},
134+
csrf_token=csrf_token,
135+
)
136+
# Now delete the author model to simulate it being missing.
137+
author_model = blog_models.BlogAuthorDetailsModel.get_by_author(
138+
new_editor_id
139+
)
140+
if author_model is None:
141+
self.fail(
142+
'Expected BlogAuthorDetailsModel to exist for user_id=%s'
143+
% new_editor_id
144+
)
145+
author_model.delete()
146+
# Verify dashboard returns default author details.
147+
json_response = self.get_json(
148+
'%s' % (feconf.BLOG_DASHBOARD_DATA_URL),
149+
)
150+
self.assertEqual(
151+
json_response['author_details']['displayed_author_name'],
152+
'Deleted User',
153+
)
154+
self.assertEqual(json_response['author_details']['author_bio'], '')
155+
self.logout()
156+
115157
def test_create_new_blog_post(self) -> None:
116158
# Checks blog editor can create a new blog post.
117159
self.login(self.BLOG_EDITOR_EMAIL)
@@ -144,12 +186,14 @@ def test_put_author_data(self) -> None:
144186

145187
pre_update_author_details = blog_services.get_blog_author_details(
146188
self.blog_editor_id
147-
).to_dict()
189+
)
190+
assert pre_update_author_details is not None
191+
pre_update_author_details_dict = pre_update_author_details.to_dict()
148192
self.assertEqual(
149-
pre_update_author_details['displayed_author_name'],
193+
pre_update_author_details_dict['displayed_author_name'],
150194
self.BLOG_EDITOR_USERNAME,
151195
)
152-
self.assertEqual(pre_update_author_details['author_bio'], '')
196+
self.assertEqual(pre_update_author_details_dict['author_bio'], '')
153197

154198
json_response = self.put_json(
155199
'%s' % (feconf.BLOG_DASHBOARD_DATA_URL),
@@ -177,9 +221,10 @@ def test_put_author_details_with_invalid_author_name(self) -> None:
177221
}
178222
pre_update_author_details = blog_services.get_blog_author_details(
179223
self.blog_editor_id
180-
).to_dict()
224+
)
225+
pre_update_author_details_dict = pre_update_author_details.to_dict()
181226
self.assertEqual(
182-
pre_update_author_details['displayed_author_name'],
227+
pre_update_author_details_dict['displayed_author_name'],
183228
self.BLOG_EDITOR_USERNAME,
184229
)
185230

@@ -196,8 +241,9 @@ def test_put_author_details_with_invalid_author_bio(self) -> None:
196241
payload = {'displayed_author_name': 'new user', 'author_bio': 1234}
197242
pre_update_author_details = blog_services.get_blog_author_details(
198243
self.blog_editor_id
199-
).to_dict()
200-
self.assertEqual(pre_update_author_details['author_bio'], '')
244+
)
245+
pre_update_author_details_dict = pre_update_author_details.to_dict()
246+
self.assertEqual(pre_update_author_details_dict['author_bio'], '')
201247

202248
self.put_json(
203249
'%s' % (feconf.BLOG_DASHBOARD_DATA_URL),
@@ -327,7 +373,6 @@ def test_get_blog_post_data_by_invalid_blog_post_id(self) -> None:
327373
def test_get_blog_post_data_with_author_account_deleted_by_blog_admin(
328374
self,
329375
) -> None:
330-
blog_services.create_blog_author_details_model(self.blog_editor_id)
331376
blog_services.update_blog_author_details(
332377
self.blog_editor_id, 'new author name', 'general user bio'
333378
)

‎core/controllers/blog_homepage.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ def _get_blog_card_summary_dicts_for_homepage(
8181
summary_dict['author_id'], strict=False
8282
)
8383
author_details = blog_services.get_blog_author_details(
84-
summary_dict['author_id']
84+
summary_dict['author_id'], strict=False
8585
)
86+
8687
if user_settings:
8788
card_summary_dict: BlogCardSummaryDict = {
8889
'id': summary_dict['id'],
@@ -275,8 +276,9 @@ def get(self, blog_post_url: str) -> None:
275276
else:
276277
author_username = 'author account deleted'
277278
author_details = blog_services.get_blog_author_details(
278-
blog_post.author_id
279+
blog_post.author_id, strict=False
279280
)
281+
280282
blog_post_dict = blog_post.to_dict()
281283
authors_blog_post_dict: AuthorsBlogPostDict = {
282284
'id': blog_post_dict['id'],
@@ -385,7 +387,7 @@ def get(self, author_username: str) -> None:
385387
% author_username
386388
)
387389

388-
author_details = blog_services.get_blog_author_details(
390+
author_details_dict = blog_services.get_blog_author_details(
389391
user_settings.user_id
390392
).to_dict()
391393
num_of_published_blog_post_summaries = blog_services.get_total_number_of_published_blog_post_summaries_by_author(
@@ -406,7 +408,7 @@ def get(self, author_username: str) -> None:
406408

407409
self.values.update(
408410
{
409-
'author_details': author_details,
411+
'author_details': author_details_dict,
410412
'no_of_blog_post_summaries': num_of_published_blog_post_summaries,
411413
'summary_dicts': blog_post_summary_dicts,
412414
}

‎core/controllers/blog_homepage_test.py‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ def test_get_blog_homepage_data(self) -> None:
112112
)
113113

114114
def test_get_blog_homepage_data_with_author_account_deleted(self) -> None:
115-
blog_services.create_blog_author_details_model(self.blog_admin_id)
116115
blog_services.update_blog_author_details(
117116
self.blog_admin_id, 'new author name', 'general user bio'
118117
)
@@ -141,6 +140,45 @@ def test_get_blog_homepage_data_with_author_account_deleted(self) -> None:
141140
'new author name',
142141
)
143142

143+
def test_pagination_with_deleted_author_on_later_pages(self) -> None:
144+
"""Tests that pagination works correctly when deleted authors appear on later pages."""
145+
self.signup('author2@example.com', 'author2')
146+
author2_id = self.get_user_id_from_email('author2@example.com')
147+
self.add_user_role('author2', feconf.ROLE_ID_BLOG_POST_EDITOR)
148+
for i in range(15):
149+
blog_post = blog_services.create_new_blog_post(self.blog_admin_id)
150+
change_dict: blog_services.BlogPostChangeDict = {
151+
'title': f'Blog Admin Post {i}',
152+
'thumbnail_filename': 'thumbnail.svg',
153+
'content': f'<p>Content {i}</p>',
154+
'tags': ['Tag1'],
155+
}
156+
blog_services.update_blog_post(blog_post.id, change_dict)
157+
blog_services.publish_blog_post(blog_post.id)
158+
for i in range(15):
159+
blog_post = blog_services.create_new_blog_post(author2_id)
160+
change_dict = {
161+
'title': f'Author2 Post {i}',
162+
'thumbnail_filename': 'thumbnail.svg',
163+
'content': f'<p>Content {i}</p>',
164+
'tags': ['Tag1'],
165+
}
166+
blog_services.update_blog_post(blog_post.id, change_dict)
167+
blog_services.publish_blog_post(blog_post.id)
168+
user_models.UserSettingsModel.delete_by_id(author2_id)
169+
self.login(self.user_email)
170+
response = self.get_json('%s?offset=0' % feconf.BLOG_HOMEPAGE_DATA_URL)
171+
self.assertEqual(len(response['blog_post_summary_dicts']), 10)
172+
response = self.get_json('%s?offset=10' % feconf.BLOG_HOMEPAGE_DATA_URL)
173+
self.assertEqual(len(response['blog_post_summary_dicts']), 10)
174+
response = self.get_json('%s?offset=20' % feconf.BLOG_HOMEPAGE_DATA_URL)
175+
self.assertEqual(len(response['blog_post_summary_dicts']), 10)
176+
for post_dict in response['blog_post_summary_dicts']:
177+
if post_dict['author_username'] == 'author account deleted':
178+
self.assertEqual(
179+
post_dict['displayed_author_name'], 'Blog Author'
180+
)
181+
144182

145183
class BlogPostDataHandlerTest(test_utils.GenericTestBase):
146184
"""Checks that the data of the blog post and other data on
@@ -167,7 +205,6 @@ def setUp(self) -> None:
167205
}
168206
blog_services.update_blog_post(self.blog_post_one.id, self.change_dict)
169207
blog_services.publish_blog_post(self.blog_post_one.id)
170-
blog_services.create_blog_author_details_model(self.blog_admin_id)
171208
blog_services.update_blog_author_details(
172209
self.blog_admin_id, 'new author name', 'general user bio'
173210
)
@@ -387,7 +424,6 @@ def setUp(self) -> None:
387424
}
388425
blog_services.update_blog_post(self.blog_post.id, self.change_dict)
389426
blog_services.publish_blog_post(self.blog_post.id)
390-
blog_services.create_blog_author_details_model(self.blog_admin_id)
391427
blog_services.update_blog_author_details(
392428
self.blog_admin_id, 'new author name', 'general user bio'
393429
)

‎core/domain/blog_domain.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,32 @@ def __init__(
748748
self.author_bio = author_bio
749749
self.last_updated = last_updated
750750

751+
@classmethod
752+
def create_default_author_details_for_user(
753+
cls, user_id: str
754+
) -> BlogAuthorDetails:
755+
"""Creates a BlogAuthorDetails with default values.
756+
757+
This is used as a fallback when the underlying
758+
BlogAuthorDetailsModel is missing (e.g., the author's account
759+
was deleted). The returned object is fully usable but contains
760+
placeholder values.
761+
762+
Args:
763+
user_id: str. The user id of the missing author.
764+
765+
Returns:
766+
BlogAuthorDetails. A domain object with default placeholder
767+
values.
768+
"""
769+
return cls(
770+
instance_id='',
771+
author_id=user_id,
772+
displayed_author_name='Deleted User',
773+
author_bio='',
774+
last_updated=datetime.datetime.utcnow(),
775+
)
776+
751777
@classmethod
752778
def require_valid_displayed_author_name(cls, author_name: str) -> None:
753779
"""Checks if the given author name is valid or not.

0 commit comments

Comments
 (0)