Skip to content

Commit

Permalink
Fix oppia#1872: Added a recently published group to library index pag…
Browse files Browse the repository at this point in the history
…e. (oppia#1934)

* Added a recently published group to library index page. oppia#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.
  • Loading branch information
souravbadami authored and seanlip committed Jun 3, 2016
1 parent d25edd4 commit 3c68fb3
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 15 deletions.
8 changes: 8 additions & 0 deletions core/controllers/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion core/domain/exp_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
5 changes: 4 additions & 1 deletion core/domain/exp_jobs_one_off_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
19 changes: 15 additions & 4 deletions core/domain/exp_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1080,15 +1088,16 @@ 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,
exploration.tags, ratings, exp_rights.status,
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

Expand Down Expand Up @@ -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()
Expand Down
9 changes: 6 additions & 3 deletions core/domain/exp_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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
)
}

Expand Down
9 changes: 3 additions & 6 deletions core/domain/rights_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
18 changes: 18 additions & 0 deletions core/domain/summary_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
123 changes: 123 additions & 0 deletions core/domain/summary_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '[email protected]'

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])
17 changes: 17 additions & 0 deletions core/storage/exploration/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)

7 changes: 7 additions & 0 deletions feconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3c68fb3

Please sign in to comment.