Skip to content

Commit

Permalink
Fix oppia#8019: Adds abbreviated topic name and thumbnail fields in… (o…
Browse files Browse the repository at this point in the history
…ppia#8067)

* adds abbreviated topic name and topic thumbnail fields

* add tests

* fix errors

* fix lint errors

* fix html lint issues

* fix html issue

* fix html lint issue

* fix failures

* fit -> it

* add tests for topic_domain andtopic_services changes

* fix lint issue

* address comments

* fix tests

* add eol

* fix test

* address review comments

* add tests for abbrev name validation

* fix backend and e2e failures

* address comments

* fixes tests; addresses comments

* fix issue with editing thumbnail

* address comments

* fix tests

* address comments

* fix lint issue

* fix frontend test failure

* change topicAndStoryEditor to run in dev mode

* address review comments; add audit job to check for empty abbreviated name fields and test

* address comments

* address remaining comments

* address review comments
  • Loading branch information
Kevin Thomas authored and vojtechjelinek committed Dec 18, 2019
1 parent ee4f41b commit 6d73c36
Show file tree
Hide file tree
Showing 69 changed files with 1,050 additions and 289 deletions.
3 changes: 2 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
/core/templates/dev/head/services/editability.service*.ts @aks681
/core/templates/dev/head/services/exploration-features*.ts @aks681
/core/templates/dev/head/services/exploration-html-formatter.service*.ts @aks681
/core/templates/dev/head/services/image-upload-helper.service*.ts @aks681
/core/templates/dev/head/services/validators.service*.ts @aks681


Expand Down Expand Up @@ -361,7 +362,7 @@
/core/domain/subtopic_page_services*.py @aks681
/core/domain/topic*.py @aks681
/core/storage/topic/ @aks681
/core/templates/dev/head/components/entity-creation-services/topic-creation.service.ts.ts @aks681
/core/templates/dev/head/components/entity-creation-services/topic-creation.service.ts @aks681
/core/templates/dev/head/domain/classroom/ @aks681
/core/templates/dev/head/domain/subtopic_viewer/ @aks681
/core/templates/dev/head/domain/topic/ @aks681
Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ script:
- if [ "$RUN_E2E_TESTS_SKILL_EDITOR" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="skillEditor" --prod_env; fi
- if [ "$RUN_E2E_TESTS_SUBSCRIPTIONS" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="subscriptions" --prod_env; fi
- if [ "$RUN_E2E_TESTS_TOPICS_AND_SKILLS_DASHBOARD" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="topicsAndSkillsDashboard" --prod_env; fi
- if [ "$RUN_E2E_TESTS_TOPIC_AND_STORY_EDITOR" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="topicAndStoryEditor" --prod_env; fi
- if [ "$RUN_E2E_TESTS_TOPIC_AND_STORY_EDITOR" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="topicAndStoryEditor"; fi
- if [ "$RUN_E2E_TESTS_USERS" == 'true' ]; then travis_retry bash scripts/run_e2e_tests.sh --suite="users" --prod_env; fi
# These lines are commented out because these checks are being run on CircleCI
# here: https://circleci.com/gh/oppia/oppia
Expand Down
24 changes: 12 additions & 12 deletions core/controllers/acl_decorators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1859,8 +1859,8 @@ def setUp(self):
debug=feconf.DEBUG,
))
self.save_new_topic(
self.topic_id, self.viewer_id, 'Name', 'Description', [], [],
[], [], 1)
self.topic_id, self.viewer_id, 'Name', 'abbrev', None,
'Description', [], [], [], [], 1)
topic_services.create_new_topic_rights(self.topic_id, self.admin_id)
topic_services.assign_role(
self.admin, self.manager, topic_domain.ROLE_MANAGER, self.topic_id)
Expand Down Expand Up @@ -1926,8 +1926,8 @@ def setUp(self):
self.story_id, self.admin_id, 'Title', 'Description', 'Notes',
self.topic_id)
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description',
[self.story_id], [], [], [], 1)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [self.story_id], [], [], [], 1)
topic_services.create_new_topic_rights(self.topic_id, self.admin_id)

def test_can_not_edit_story_with_invalid_story_id(self):
Expand Down Expand Up @@ -2015,8 +2015,8 @@ def setUp(self):
debug=feconf.DEBUG,
))
self.save_new_topic(
self.topic_id, self.viewer_id, 'Name', 'Description', [], [],
[], [], 1)
self.topic_id, self.viewer_id, 'Name', 'abbrev', None,
'Description', [], [], [], [], 1)
topic_services.create_new_topic_rights(self.topic_id, self.admin_id)
topic_services.assign_role(
self.admin, self.manager, topic_domain.ROLE_MANAGER, self.topic_id)
Expand Down Expand Up @@ -2099,8 +2099,8 @@ def setUp(self):
self.story_id, self.admin_id, 'Title', 'Description', 'Notes',
self.topic_id)
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description',
[self.story_id], [], [], [], 1)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [self.story_id], [], [], [], 1)

def test_cannot_access_non_existent_story(self):
with self.swap(self, 'testapp', self.mock_testapp):
Expand Down Expand Up @@ -2823,8 +2823,8 @@ def test_can_edit_topic(self):
self.login(self.ADMIN_EMAIL)
topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
topic_id, self.admin_id, 'Name', 'Description',
[], [], [], [], 1)
topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [], [], [], [], 1)
with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json('/mock_edit_entity/%s/%s' % (
feconf.ENTITY_TYPE_TOPIC, topic_id))
Expand All @@ -2851,8 +2851,8 @@ def test_can_edit_story(self):
story_id, self.admin_id, 'Title', 'Description', 'Notes',
topic_id)
self.save_new_topic(
topic_id, self.admin_id, 'Name', 'Description',
[story_id], [], [], [], 1)
topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [story_id], [], [], [], 1)
with self.swap(self, 'testapp', self.mock_testapp):
response = self.get_json('/mock_edit_entity/%s/%s' % (
feconf.ENTITY_TYPE_STORY, story_id))
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,9 @@ def _load_dummy_new_structures_data(self):
self.user_id, question_id_3, skill_id_3, 0.7)

topic_1 = topic_domain.Topic.create_default_topic(
topic_id_1, 'Dummy Topic 1')
topic_id_1, 'Dummy Topic 1', 'abbrev')
topic_2 = topic_domain.Topic.create_default_topic(
topic_id_2, 'Empty Topic')
topic_id_2, 'Empty Topic', 'abbrev')

topic_1.add_canonical_story(story_id)
topic_1.add_uncategorized_skill_id(skill_id_1)
Expand Down
5 changes: 3 additions & 2 deletions core/controllers/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def test_regenerate_topic_related_opportunities_action(self):
exp_services.save_new_exploration(owner_id, exploration)

topic = topic_domain.Topic.create_default_topic(
topic_id=topic_id, name='topic')
topic_id=topic_id, name='topic', abbreviated_name='abbrev')
topic_services.save_new_topic(owner_id, topic)

story = story_domain.Story.create_default_story(
Expand Down Expand Up @@ -872,7 +872,8 @@ def test_changing_user_role_from_exploration_editor_to_topic_manager(self):

topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
topic_id, user_id, 'Name', 'Description', [], [], [], [], 1)
topic_id, user_id, 'Name', 'abbrev', None,
'Description', [], [], [], [], 1)

self.login(self.ADMIN_EMAIL, is_super_admin=True)

Expand Down
4 changes: 2 additions & 2 deletions core/controllers/classroom_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ def test_get(self):
topic_id_1 = topic_services.get_new_topic_id()
topic_id_2 = topic_services.get_new_topic_id()
private_topic = topic_domain.Topic.create_default_topic(
topic_id_1, 'private_topic_name')
topic_id_1, 'private_topic_name', 'abbrev')
topic_services.save_new_topic(admin_id, private_topic)
public_topic = topic_domain.Topic.create_default_topic(
topic_id_2, 'public_topic_name')
topic_id_2, 'public_topic_name', 'abbrev')
topic_services.save_new_topic(admin_id, public_topic)
topic_services.publish_topic(topic_id_2, admin_id)

Expand Down
4 changes: 2 additions & 2 deletions core/controllers/community_dashboard_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def setUp(self):
exp_services.save_new_exploration(self.owner_id, exp)

topic = topic_domain.Topic.create_default_topic(
topic_id='0', name='topic')
topic_id='0', name='topic', abbreviated_name='abbrev')
topic_services.save_new_topic(self.owner_id, topic)

self.skill_id_0 = 'skill_id_0'
Expand Down Expand Up @@ -317,7 +317,7 @@ def setUp(self):
exp_services.save_new_exploration(self.owner_id, exp)

topic = topic_domain.Topic.create_default_topic(
topic_id='0', name='topic')
topic_id='0', name='topic', abbreviated_name='abbrev')
topic_services.save_new_topic(self.owner_id, topic)

stories = [story_domain.Story.create_default_story(
Expand Down
6 changes: 3 additions & 3 deletions core/controllers/creator_dashboard_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,9 +709,9 @@ def test_get_topic_summary_dicts_with_new_structure_players_enabled(self):
response = self.get_json(feconf.CREATOR_DASHBOARD_DATA_URL)
self.assertEqual(len(response['topic_summary_dicts']), 0)
self.save_new_topic(
'topic_id', self.owner_id, 'Name', 'Description',
['story_id_1', 'story_id_2'], ['story_id_3'],
['skill_id_1', 'skill_id_2'], [], 1)
'topic_id', self.owner_id, 'Name', 'abbrev', None,
'Description', ['story_id_1', 'story_id_2'],
['story_id_3'], ['skill_id_1', 'skill_id_2'], [], 1)
response = self.get_json(feconf.CREATOR_DASHBOARD_DATA_URL)
self.assertEqual(len(response['topic_summary_dicts']), 1)
self.assertTrue(isinstance(response['topic_summary_dicts'], list))
Expand Down
8 changes: 6 additions & 2 deletions core/controllers/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,9 @@ def post(self, entity_type, entity_id):

raw = self.request.get('image')
filename = self.payload.get('filename')
filename_prefix = self.payload.get('filename_prefix')
if filename_prefix is None:
filename_prefix = self._FILENAME_PREFIX
if not raw:
raise self.InvalidInputException('No image supplied')

Expand Down Expand Up @@ -659,15 +662,16 @@ def post(self, entity_type, entity_id):
file_system_class = fs_services.get_entity_file_system_class()
fs = fs_domain.AbstractFileSystem(file_system_class(
entity_type, entity_id))
filepath = '%s/%s' % (self._FILENAME_PREFIX, filename)
filepath = '%s/%s' % (filename_prefix, filename)

if fs.isfile(filepath):
raise self.InvalidInputException(
'A file with the name %s already exists. Please choose a '
'different name.' % filename)

fs_services.save_original_and_compressed_versions_of_image(
self.user_id, filename, entity_type, entity_id, raw)
self.user_id, filename, entity_type, entity_id,
raw, filename_prefix)

self.render_json({'filename': filename})

Expand Down
4 changes: 2 additions & 2 deletions core/controllers/feedback_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,8 @@ def setUp(self):

self.topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
self.topic_id, self.owner_id, 'Name', 'Description',
[], [], [], [], 1)
self.topic_id, self.owner_id, 'Name', 'abbrev', None,
'Description', [], [], [], [], 1)

def test_get_feedback_threads_linked_to_topics(self):
self.login(self.OWNER_EMAIL)
Expand Down
6 changes: 3 additions & 3 deletions core/controllers/practice_sessions_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ def setUp(self):
self.save_new_skill(self.skill_id2, self.admin_id, 'Skill 2')

self.topic = topic_domain.Topic.create_default_topic(
self.topic_id, 'public_topic_name')
self.topic_id, 'public_topic_name', 'abbrev')
self.topic.uncategorized_skill_ids.append(self.skill_id1)
self.topic.subtopics.append(topic_domain.Subtopic(
1, 'subtopic_name', [self.skill_id2]))
self.topic.next_subtopic_id = 2
topic_services.save_new_topic(self.admin_id, self.topic)

self.topic = topic_domain.Topic.create_default_topic(
self.topic_id_1, 'private_topic_name')
self.topic_id_1, 'private_topic_name', 'abbrev')
topic_services.save_new_topic(self.admin_id, self.topic)

topic_services.publish_topic(self.topic_id, self.admin_id)
Expand Down Expand Up @@ -104,7 +104,7 @@ def test_get_fails_when_new_structures_not_enabled(self):

def test_get_fails_when_skill_ids_dont_exist(self):
topic = topic_domain.Topic.create_default_topic(
'topic_id_3', 'topic_without_skills')
'topic_id_3', 'topic_without_skills', 'abbrev')
topic.uncategorized_skill_ids.append('non_existent_skill')
topic_services.save_new_topic(self.admin_id, topic)
topic_services.publish_topic('topic_id_3', self.admin_id)
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/questions_list_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def setUp(self):
self.skill_id_2, self.admin_id, 'Skill Description 2')
self.topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description', [], [],
[self.skill_id, self.skill_id_2], [], 1)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [], [], [self.skill_id, self.skill_id_2], [], 1)
self.skill_id_3 = skill_services.get_new_skill_id()
changelist = [topic_domain.TopicChange({
'cmd': topic_domain.CMD_ADD_SUBTOPIC,
Expand Down
3 changes: 2 additions & 1 deletion core/controllers/reader_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ def test_get_exploration_pretests(self):
story_id = story_services.get_new_story_id()
topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
topic_id, 'user', 'Topic', 'A new topic', [], [], [], [], 0)
topic_id, 'user', 'Topic', 'abbrev', None, 'A new topic',
[], [], [], [], 0)
self.save_new_story(
story_id, 'user', 'Title', 'Description', 'Notes', topic_id
)
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class AssetDevHandler(base.BaseHandler):
image and audio files are served from GCS).
"""

_SUPPORTED_TYPES = ['image', 'audio']
_SUPPORTED_TYPES = ['image', 'audio', 'thumbnail']
_SUPPORTED_PAGE_CONTEXTS = [
feconf.ENTITY_TYPE_EXPLORATION, feconf.ENTITY_TYPE_SKILL,
feconf.ENTITY_TYPE_TOPIC, feconf.ENTITY_TYPE_STORY,
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/resources_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ def test_image_upload_and_download(self):
story_id, admin_id, 'Title', 'Description', 'Notes',
topic_id)
self.save_new_topic(
topic_id, admin_id, 'Name', 'Description',
[story_id], [], [], [subtopic], 2)
topic_id, admin_id, 'Name', 'abbrev', None,
'Description', [story_id], [], [], [subtopic], 2)
self.save_new_skill(skill_id, admin_id, 'Description')

# Page context: Exploration.
Expand Down
5 changes: 3 additions & 2 deletions core/controllers/review_tests_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ def setUp(self):
self.story_id_2, 'Private Story Title', self.topic_id)
story_services.save_new_story(self.admin_id, self.story_2)
self.save_new_topic(
self.topic_id, 'user', 'Topic', 'A new topic',
[self.story_id_1, self.story_id_3], [], [], [], 0)
self.topic_id, 'user', 'Topic', 'abbrev', None,
'A new topic', [self.story_id_1, self.story_id_3],
[], [], [], 0)
topic_services.publish_topic(self.topic_id, self.admin_id)
topic_services.publish_story(
self.topic_id, self.story_id_1, self.admin_id)
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/skill_editor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ def setUp(self):
self.save_new_skill(self.skill_id_2, self.admin_id, 'Description')
self.topic_id = topic_services.get_new_topic_id()
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description',
[], [], [self.skill_id], [], 1)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [], [], [self.skill_id], [], 1)

def delete_skill_model_and_memcache(self, user_id, skill_id):
"""Deletes skill model and memcache corresponding to the given skill
Expand Down
12 changes: 6 additions & 6 deletions core/controllers/story_editor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def setUp(self):
self.story_id, self.admin_id, 'Title', 'Description', 'Notes',
self.topic_id)
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description',
[self.story_id], [], [], [], 1)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [self.story_id], [], [], [], 1)


class StoryPublicationTests(BaseStoryEditorControllerTests):
Expand Down Expand Up @@ -186,8 +186,8 @@ def test_can_not_get_access_story_handler_with_invalid_topic_id(self):
expected_status_int=404)

self.save_new_topic(
'topic_id_new', self.admin_id, 'Name 2', 'Description',
[], [], [], [], 1)
'topic_id_new', self.admin_id, 'Name 2', 'abbrev', None,
'Description', [], [], [], [], 1)

# An error would be raised here also as the story is not in the given
# topic.
Expand Down Expand Up @@ -260,8 +260,8 @@ def test_put_can_not_access_story_handler_with_invalid_topic_id(self):
# Raises error 404 even when topic is saved as the story id is not
# associated with the new topic.
self.save_new_topic(
'topic_id_new', self.admin_id, 'Name 2', 'Description',
[], [], [], [], 1)
'topic_id_new', self.admin_id, 'Name 2', 'abbrev', None,
'Description', [], [], [], [], 1)
csrf_token = self.get_new_csrf_token()

self.put_json(
Expand Down
8 changes: 4 additions & 4 deletions core/controllers/story_viewer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ def setUp(self):
story.story_contents.next_node_id = 'node_4'
story_services.save_new_story(self.admin_id, story)
self.save_new_topic(
self.TOPIC_ID, 'user', 'Topic', 'A new topic', [story.id],
[], [], [], 0)
self.TOPIC_ID, 'user', 'Topic', 'abbrev', None,
'A new topic', [story.id], [], [], [], 0)
topic_services.publish_topic(self.TOPIC_ID, self.admin_id)
topic_services.publish_story(
self.TOPIC_ID, self.STORY_ID_1, self.admin_id)
Expand Down Expand Up @@ -169,8 +169,8 @@ def test_can_not_access_story_viewer_page_with_unpublished_story(self):
def test_can_not_access_story_viewer_page_with_unpublished_topic(self):
new_story_id = 'new_story_id'
self.save_new_topic(
'topic_id_1', 'user', 'Topic 2', 'A new topic', [new_story_id],
[], [], [], 0)
'topic_id_1', 'user', 'Topic 2', 'abbrev', None,
'A new topic', [new_story_id], [], [], [], 0)
story = story_domain.Story.create_default_story(
new_story_id, 'Title', 'topic_id_1')
story_services.save_new_story(self.admin_id, story)
Expand Down
8 changes: 4 additions & 4 deletions core/controllers/subtopic_viewer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ def setUp(self):
subtopic = topic_domain.Subtopic.create_default_subtopic(
1, 'Subtopic Title')
self.save_new_topic(
self.topic_id, self.admin_id, 'Name', 'Description', [], [],
[], [subtopic], 2)
self.topic_id, self.admin_id, 'Name', 'abbrev', None,
'Description', [], [], [], [subtopic], 2)
topic_services.publish_topic(self.topic_id, self.admin_id)
self.save_new_topic(
'topic_id_2', self.admin_id, 'Private_Name', 'Description', [], [],
[], [subtopic], 2)
'topic_id_2', self.admin_id, 'Private_Name', 'abbrev', None,
'Description', [], [], [], [subtopic], 2)
self.recorded_voiceovers_dict = {
'voiceovers_mapping': {
'content': {
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/suggestion_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ def setUp(self):
exp_services.save_new_exploration(self.owner_id, exploration)

topic = topic_domain.Topic.create_default_topic(
topic_id=self.TOPIC_ID, name='topic')
topic_id=self.TOPIC_ID, name='topic', abbreviated_name='abbrev')
topic_services.save_new_topic(self.owner_id, topic)

story = story_domain.Story.create_default_story(
Expand Down Expand Up @@ -1145,7 +1145,7 @@ def setUp(self):
exp_services.save_new_exploration(self.owner_id, exploration)

topic = topic_domain.Topic.create_default_topic(
topic_id=self.TOPIC_ID, name='topic')
topic_id=self.TOPIC_ID, name='topic', abbreviated_name='abbrev')
topic_services.save_new_topic(self.owner_id, topic)

story = story_domain.Story.create_default_story(
Expand Down
Loading

0 comments on commit 6d73c36

Please sign in to comment.