From 3c68fb3a7c74d3ed4acdd442240c6df4bbc69e07 Mon Sep 17 00:00:00 2001 From: Sourav Badami Date: Fri, 3 Jun 2016 14:02:37 +0530 Subject: [PATCH] Fix #1872: Added a recently published group to library index page. (#1934) * Added a recently published group to library index page. #1872 * Added datastore query to extract explorations for recently published category. * Modified get_recently_published_exploration_summary_dicts to get explorations regardless of language code and some minor changes. * Modified summary_services_test to add a case for edited explorations. * Solved merge conflicts. * Nit. * Minor changes. * Added comments. * Fixed linting issues. * Minor changes. --- core/controllers/library.py | 8 ++ core/domain/exp_domain.py | 4 +- core/domain/exp_jobs_one_off_test.py | 5 +- core/domain/exp_services.py | 19 +++- core/domain/exp_services_test.py | 9 +- core/domain/rights_manager.py | 9 +- core/domain/summary_services.py | 18 ++++ core/domain/summary_services_test.py | 123 +++++++++++++++++++++++++ core/storage/exploration/gae_models.py | 17 ++++ feconf.py | 7 ++ index.yaml | 7 ++ 11 files changed, 211 insertions(+), 15 deletions(-) diff --git a/core/controllers/library.py b/core/controllers/library.py index 1bfc45773aa7..57f8b41c99cb 100644 --- a/core/controllers/library.py +++ b/core/controllers/library.py @@ -85,6 +85,8 @@ def get(self): # TODO(sll): Support index pages for other language codes. summary_dicts_by_category = summary_services.get_library_groups([ feconf.DEFAULT_LANGUAGE_CODE]) + recently_published_summary_dicts = ( + summary_services.get_recently_published_exploration_summary_dicts()) top_rated_activity_summary_dicts = ( summary_services.get_top_rated_exploration_summary_dicts( [feconf.DEFAULT_LANGUAGE_CODE])) @@ -97,6 +99,12 @@ def get(self): user_settings = user_services.get_user_settings(self.user_id) preferred_language_codes = user_settings.preferred_language_codes + if recently_published_summary_dicts: + summary_dicts_by_category.insert(0, { + 'activity_summary_dicts': recently_published_summary_dicts, + 'categories': [], + 'header': feconf.LIBRARY_CATEGORY_RECENTLY_PUBLISHED, + }) if top_rated_activity_summary_dicts: summary_dicts_by_category.insert(0, { 'activity_summary_dicts': top_rated_activity_summary_dicts, diff --git a/core/domain/exp_domain.py b/core/domain/exp_domain.py index df762cfbdacc..40a4a2b3c234 100644 --- a/core/domain/exp_domain.py +++ b/core/domain/exp_domain.py @@ -2586,7 +2586,8 @@ def __init__(self, exploration_id, title, category, objective, community_owned, owner_ids, editor_ids, viewer_ids, contributor_ids, contributors_summary, version, exploration_model_created_on, - exploration_model_last_updated): + exploration_model_last_updated, + first_published_msec): """'ratings' is a dict whose keys are '1', '2', '3', '4', '5' and whose values are nonnegative integers representing frequency counts. Note that the keys need to be strings in order for this dict to be @@ -2609,3 +2610,4 @@ def __init__(self, exploration_id, title, category, objective, self.version = version self.exploration_model_created_on = exploration_model_created_on self.exploration_model_last_updated = exploration_model_last_updated + self.first_published_msec = first_published_msec diff --git a/core/domain/exp_jobs_one_off_test.py b/core/domain/exp_jobs_one_off_test.py index 907e97bd0b57..6222dab066b1 100644 --- a/core/domain/exp_jobs_one_off_test.py +++ b/core/domain/exp_jobs_one_off_test.py @@ -167,6 +167,8 @@ def _run_batch_job_once_and_verify_output( exploration = exp_services.get_exploration_by_id(exp_id) exploration_model_last_updated = exploration.last_updated exploration_model_created_on = exploration.created_on + first_published_msec = ( + exp_rights_model.first_published_msec) # Manually create the expected summary specifying title, # category, etc. @@ -187,7 +189,8 @@ def _run_batch_job_once_and_verify_output( {admin_id: 1}, exploration.version, exploration_model_created_on, - exploration_model_last_updated) + exploration_model_last_updated, + first_published_msec) # Note: Calling constructor for fields that are not required # and have no default value does not work, because diff --git a/core/domain/exp_services.py b/core/domain/exp_services.py index 10a78e1aee0b..acf76f7876aa 100644 --- a/core/domain/exp_services.py +++ b/core/domain/exp_services.py @@ -165,7 +165,8 @@ def get_exploration_summary_from_model(exp_summary_model): exp_summary_model.contributor_ids, exp_summary_model.contributors_summary, exp_summary_model.version, exp_summary_model.exploration_model_created_on, - exp_summary_model.exploration_model_last_updated + exp_summary_model.exploration_model_last_updated, + exp_summary_model.first_published_msec ) @@ -374,6 +375,13 @@ def get_featured_exploration_summaries(): return _get_exploration_summaries_from_models( exp_models.ExpSummaryModel.get_featured()) +def get_recently_published_exploration_summaries(): + """Returns a dict with all featured exploration summary domain objects, + keyed by their id. + """ + return _get_exploration_summaries_from_models( + exp_models.ExpSummaryModel.get_recently_published()) + def get_all_exploration_summaries(): """Returns a dict with all exploration summary domain objects, @@ -1080,7 +1088,7 @@ def compute_summary_of_exploration(exploration, contributor_id_to_add): exploration_model_last_updated = datetime.datetime.fromtimestamp( _get_last_updated_by_human_ms(exploration.id) / 1000.0) exploration_model_created_on = exploration.created_on - + first_published_msec = exp_rights.first_published_msec exp_summary = exp_domain.ExplorationSummary( exploration.id, exploration.title, exploration.category, exploration.objective, exploration.language_code, @@ -1088,7 +1096,8 @@ def compute_summary_of_exploration(exploration, contributor_id_to_add): exp_rights.community_owned, exp_rights.owner_ids, exp_rights.editor_ids, exp_rights.viewer_ids, contributor_ids, contributors_summary, exploration.version, - exploration_model_created_on, exploration_model_last_updated) + exploration_model_created_on, exploration_model_last_updated, + first_published_msec) return exp_summary @@ -1141,7 +1150,9 @@ def save_exploration_summary(exp_summary): exploration_model_last_updated=( exp_summary.exploration_model_last_updated), exploration_model_created_on=( - exp_summary.exploration_model_created_on) + exp_summary.exploration_model_created_on), + first_published_msec=( + exp_summary.first_published_msec) ) exp_summary_model.put() diff --git a/core/domain/exp_services_test.py b/core/domain/exp_services_test.py index 6e54d6e3dfc2..35996b14f3ba 100644 --- a/core/domain/exp_services_test.py +++ b/core/domain/exp_services_test.py @@ -2269,7 +2269,8 @@ def test_get_non_private_exploration_summaries(self): {self.albert_id: 1}, self.EXPECTED_VERSION_2, actual_summaries[self.EXP_ID_2].exploration_model_created_on, - actual_summaries[self.EXP_ID_2].exploration_model_last_updated + actual_summaries[self.EXP_ID_2].exploration_model_last_updated, + actual_summaries[self.EXP_ID_2].first_published_msec )} # check actual summaries equal expected summaries @@ -2299,7 +2300,8 @@ def test_get_all_exploration_summaries(self): False, [self.albert_id], [], [], [self.albert_id, self.bob_id], {self.albert_id: 1, self.bob_id: 1}, self.EXPECTED_VERSION_1, actual_summaries[self.EXP_ID_1].exploration_model_created_on, - actual_summaries[self.EXP_ID_1].exploration_model_last_updated + actual_summaries[self.EXP_ID_1].exploration_model_last_updated, + actual_summaries[self.EXP_ID_1].first_published_msec ), self.EXP_ID_2: exp_domain.ExplorationSummary( self.EXP_ID_2, 'Exploration 2 Albert title', @@ -2309,7 +2311,8 @@ def test_get_all_exploration_summaries(self): False, [self.albert_id], [], [], [self.albert_id], {self.albert_id: 1}, self.EXPECTED_VERSION_2, actual_summaries[self.EXP_ID_2].exploration_model_created_on, - actual_summaries[self.EXP_ID_2].exploration_model_last_updated + actual_summaries[self.EXP_ID_2].exploration_model_last_updated, + actual_summaries[self.EXP_ID_2].first_published_msec ) } diff --git a/core/domain/rights_manager.py b/core/domain/rights_manager.py index 9cfe071efa70..4c70e7a85ca9 100644 --- a/core/domain/rights_manager.py +++ b/core/domain/rights_manager.py @@ -625,18 +625,15 @@ def _change_activity_status( if new_status != ACTIVITY_STATUS_PRIVATE: activity_rights.viewer_ids = [] + if activity_rights.first_published_msec is None: + activity_rights.first_published_msec = ( + utils.get_current_time_in_millisecs()) _save_activity_rights( committer_id, activity_rights, activity_type, commit_message, commit_cmds) _update_activity_summary(activity_type, activity_rights) - if new_status != ACTIVITY_STATUS_PRIVATE: - # Change first_published_msec in activity rights if necessary. - if activity_rights.first_published_msec is None: - activity_rights.first_published_msec = ( - utils.get_current_time_in_millisecs()) - def _publish_activity(committer_id, activity_id, activity_type): if not Actor(committer_id).can_publish(activity_type, activity_id): diff --git a/core/domain/summary_services.py b/core/domain/summary_services.py index 78c97895e18b..0b108460dea8 100644 --- a/core/domain/summary_services.py +++ b/core/domain/summary_services.py @@ -396,3 +396,21 @@ def get_top_rated_exploration_summary_dicts(language_codes): return _get_displayable_exp_summary_dicts( sorted_exp_summaries, include_contributors=False) + + +def get_recently_published_exploration_summary_dicts(): + """Returns a list of recently published explorations + with the given language code. + """ + recently_published_exploration_summaries = [ + exp_summary for exp_summary in + exp_services.get_recently_published_exploration_summaries().values()] + + # Arranging recently published exploration summaries with respect to time. + # sorted() is used to sort the random list of recently published summaries. + summaries = sorted( + recently_published_exploration_summaries, + key=lambda exp_summary: exp_summary.first_published_msec, + reverse=True) + + return _get_displayable_exp_summary_dicts(summaries) diff --git a/core/domain/summary_services_test.py b/core/domain/summary_services_test.py index cf7425db19d7..6f6b68439d9d 100644 --- a/core/domain/summary_services_test.py +++ b/core/domain/summary_services_test.py @@ -641,3 +641,126 @@ def test_only_explorations_with_ratings_are_returned(self): top_rated_exploration_summaries] self.assertEqual(expected_ordering, actual_ordering) + + +class RecentlyPublishedExplorationDisplayableSummariesTest( + test_utils.GenericTestBase): + """Test functions for getting displayable recently published exploration + summary dicts. + """ + + ALBERT_NAME = 'albert' + ALBERT_EMAIL = 'albert@example.com' + + EXP_ID_1 = 'eid1' + EXP_ID_2 = 'eid2' + EXP_ID_3 = 'eid3' + + def setUp(self): + """Populate the database of explorations and their summaries. + + The sequence of events is: + - (1) Albert creates EXP_ID_1. + - (2) Albert creates EXP_ID_2. + - (3) Albert creates EXP_ID_3. + - (4) Albert publishes EXP_ID_1. + - (5) Albert publishes EXP_ID_2. + - (6) Albert publishes EXP_ID_3. + - (7) Admin user is set up. + """ + + super(RecentlyPublishedExplorationDisplayableSummariesTest, + self).setUp() + + self.admin_id = self.get_user_id_from_email(self.ADMIN_EMAIL) + self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL) + self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME) + self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME) + + self.save_new_valid_exploration( + self.EXP_ID_1, self.albert_id, + end_state_name='End') + self.save_new_valid_exploration( + self.EXP_ID_2, self.albert_id, + end_state_name='End') + self.save_new_valid_exploration( + self.EXP_ID_3, self.albert_id, + end_state_name='End') + + rights_manager.publish_exploration(self.albert_id, self.EXP_ID_2) + rights_manager.publish_exploration(self.albert_id, self.EXP_ID_1) + rights_manager.publish_exploration(self.albert_id, self.EXP_ID_3) + + self.set_admins([self.ADMIN_USERNAME]) + + def test_for_recently_published_explorations(self): + """ Tests for recently published explorations. + """ + + recently_published_exploration_summaries = ( + summary_services.get_recently_published_exploration_summary_dicts()) + test_summary_1 = { + 'status': 'public', + 'thumbnail_bg_color': '#a33f40', + 'community_owned': False, + 'tags': [], + 'thumbnail_icon_url': '/images/subjects/Lightbulb.svg', + 'language_code': feconf.DEFAULT_LANGUAGE_CODE, + 'id': self.EXP_ID_1, + 'category': u'A category', + 'ratings': feconf.get_empty_ratings(), + 'title': u'A title', + 'num_views': 0, + 'objective': u'An objective' + } + test_summary_2 = { + 'status': 'public', + 'thumbnail_bg_color': '#a33f40', + 'community_owned': False, + 'tags': [], + 'thumbnail_icon_url': '/images/subjects/Lightbulb.svg', + 'language_code': feconf.DEFAULT_LANGUAGE_CODE, + 'id': self.EXP_ID_2, + 'category': u'A category', + 'ratings': feconf.get_empty_ratings(), + 'title': u'A title', + 'num_views': 0, + 'objective': u'An objective' + } + test_summary_3 = { + 'status': 'public', + 'thumbnail_bg_color': '#a33f40', + 'community_owned': False, + 'tags': [], + 'thumbnail_icon_url': '/images/subjects/Lightbulb.svg', + 'language_code': feconf.DEFAULT_LANGUAGE_CODE, + 'id': self.EXP_ID_3, + 'category': u'A category', + 'ratings': feconf.get_empty_ratings(), + 'title': u'A title', + 'num_views': 0, + 'objective': u'An objective' + } + + self.assertDictContainsSubset( + test_summary_3, recently_published_exploration_summaries[0]) + self.assertDictContainsSubset( + test_summary_1, recently_published_exploration_summaries[1]) + self.assertDictContainsSubset( + test_summary_2, recently_published_exploration_summaries[2]) + + # Test that editing an exploration does not change its + # 'recently-published' status. + exp_services.update_exploration( + self.albert_id, self.EXP_ID_1, [{ + 'cmd': 'edit_exploration_property', + 'property_name': 'title', + 'new_value': 'New title' + }], 'Changed title.') + + recently_published_exploration_summaries = ( + summary_services.get_recently_published_exploration_summary_dicts()) + self.assertEqual( + recently_published_exploration_summaries[1]['title'], 'New title') + self.assertDictContainsSubset( + test_summary_3, recently_published_exploration_summaries[0]) diff --git a/core/storage/exploration/gae_models.py b/core/storage/exploration/gae_models.py index a95c181b3ed0..3c877d0c02ff 100644 --- a/core/storage/exploration/gae_models.py +++ b/core/storage/exploration/gae_models.py @@ -325,6 +325,8 @@ class ExpSummaryModel(base_models.BaseModel): # with created_on, which is the time when the exploration *summary* # model was created) exploration_model_created_on = ndb.DateTimeProperty(indexed=True) + # Time when the exploration was first published. + first_published_msec = ndb.FloatProperty(indexed=True) # The publication status of this exploration. status = ndb.StringProperty( @@ -400,3 +402,18 @@ def get_at_least_editable(cls, user_id): ).filter( ExpSummaryModel.deleted == False # pylint: disable=singleton-comparison ).fetch(feconf.DEFAULT_QUERY_LIMIT) + + @classmethod + def get_recently_published(cls): + """Returns an iterable with exp summaries that are recently + published. + """ + return ExpSummaryModel.query().filter( + ndb.OR(ExpSummaryModel.status == feconf.ACTIVITY_STATUS_PUBLIC, + ExpSummaryModel.status == feconf.ACTIVITY_STATUS_PUBLICIZED) + ).filter( + ExpSummaryModel.deleted == False # pylint: disable=singleton-comparison + ).order( + -ExpSummaryModel.first_published_msec + ).fetch(feconf.RECENTLY_PUBLISHED_QUERY_LIMIT) + diff --git a/feconf.py b/feconf.py index 904b17e369d8..b5d74f81b8bb 100644 --- a/feconf.py +++ b/feconf.py @@ -54,6 +54,10 @@ # The maximum number of results to retrieve in a datastore query. DEFAULT_QUERY_LIMIT = 1000 +# The maximum number of results to retrieve in a datastore query +# for recently published explorations. +RECENTLY_PUBLISHED_QUERY_LIMIT = 8 + # The current version of the exploration states blob schema. If any backward- # incompatible changes are made to the states blob schema in the data store, # this version number must be changed and the exploration migration job @@ -528,6 +532,9 @@ def get_empty_ratings(): # The header for the "Top Rated Explorations" category in the library index # page. LIBRARY_CATEGORY_TOP_RATED_EXPLORATIONS = 'Top Rated Explorations' +# The header for the "Recently Published" category in the library index +# page. +LIBRARY_CATEGORY_RECENTLY_PUBLISHED = 'Recently Published' # List of supported language codes. Each description has a # parenthetical part that may be stripped out to give a shorter diff --git a/index.yaml b/index.yaml index 065715c2a478..14f00887af04 100644 --- a/index.yaml +++ b/index.yaml @@ -30,6 +30,13 @@ indexes: - name: deleted - name: status +- kind: ExpSummaryModel + properties: + - name: deleted + - name: status + - name: first_published_msec + direction: desc + - kind: ExpSummaryRealtimeModel properties: - name: realtime_layer