Skip to content

Commit

Permalink
Addressed code review, only minor changes.
Browse files Browse the repository at this point in the history
  • Loading branch information
mschmittfull committed Sep 30, 2014
1 parent 643f625 commit ab9cd67
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 94 deletions.
13 changes: 10 additions & 3 deletions core/controllers/galleries.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
models.NAMES.base_model, models.NAMES.exploration])
current_user_services = models.Registry.import_current_user_services()
import feconf
import utils

import jinja2

Expand Down Expand Up @@ -81,7 +82,10 @@ def _get_short_language_description(self, full_language_description):
def get(self):
"""Handles GET requests."""
# TODO(sll): Implement paging.
# TODO(msl): Add realtime layer to get up-to-date gallery

# TODO(sll): Precompute and cache gallery categories. Or have a fixed
# list of categories and 'Other', and gradually classify the
# explorations in 'Other'.

language_codes_to_short_descs = {
lc['code']: self._get_short_language_description(lc['description'])
Expand All @@ -95,20 +99,23 @@ def get(self):
exp_services.get_private_at_least_viewable_exploration_summaries(
self.user_id))

# TODO(msl): Store 'is_editable' in exploration summary to avoid O(n)
# individual lookups. Note that this will depend on user_id.
explorations_list = [{
'id': exp_summary.id,
'title': exp_summary.title,
'category': exp_summary.category,
'objective': exp_summary.objective,
'language': language_codes_to_short_descs.get(
exp_summary.language_code, exp_summary.language_code),
'last_updated': exp_summary.last_updated,
'last_updated': utils.get_time_in_millisecs(
exp_summary.last_updated),
'status': exp_summary.status,
'community_owned': exp_summary.community_owned,
'is_editable': exp_services.exp_summary_is_editable(
exp_summary,
user_id=self.user_id)
} for key, exp_summary in exp_summaries_dict.iteritems()]
} for exp_summary in exp_summaries_dict.values()]

if len(explorations_list) == feconf.DEFAULT_QUERY_LIMIT:
logging.error(
Expand Down
1 change: 0 additions & 1 deletion core/domain/exp_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,6 @@ def is_equal_to(self, other):

for prop in simple_props:
if getattr(self, prop) != getattr(other, prop):
print 'different', prop, getattr(self, prop), getattr(other, prop)
return False

return True
15 changes: 6 additions & 9 deletions core/domain/exp_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@

import ast

from core.domain import exp_domain
import feconf
from core import jobs
from core.domain import exp_domain
from core.platform import models
(base_models, exp_models,) = models.Registry.import_models([
models.NAMES.base_model, models.NAMES.exploration])
Expand All @@ -38,14 +38,15 @@ class ExpSummaryRealtimeModel(
jobs.BaseRealtimeDatastoreClassForContinuousComputations):
pass


class ExpSummariesAggregator(jobs.BaseContinuousComputationManager):
"""A continuous-computation job computing summaries of all explorations.
The summaries store the following information:
title, category, objective, language_code, skill_tags,
last_updated (as float in milliseconds), created_on (as
float in milliseconds), status (private, public or publicized),
community_owned, owner_ids, editor_ids, viewer_ids, version.
title, category, objective, language_code, skill_tags,
last_updated, created_on, status (private, public or
publicized), community_owned, owner_ids, editor_ids,
viewer_ids, version.
"""
@classmethod
def get_event_types_listened_to(cls):
Expand Down Expand Up @@ -84,18 +85,14 @@ def map(exploration_model):
from core.domain import exp_services
if ExpSummaryMRJobManager._entity_created_before_job_queued(
exploration_model):
# create exploration summary
exploration = exp_services.get_exploration_from_model(exploration_model)
exp_services.create_exploration_summary(exploration)
yield (exploration_model.id, None)

@staticmethod
def reduce(exp_id, list_of_exps):
pass




class IndexAllExplorationsJobManager(jobs.BaseMapReduceJobManager):
"""Job that indexes all explorations"""

Expand Down
6 changes: 3 additions & 3 deletions core/domain/exp_jobs_for_production_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@

__author__ = 'Marcel Schmittfull'

from core.domain import exp_domain
import feconf
from core import jobs
from core.domain import exp_domain
from core.platform import models
(base_models, exp_models,) = models.Registry.import_models([
models.NAMES.base_model, models.NAMES.exploration])
#transaction_services = models.Registry.import_transaction_services()
#import utils

from google.appengine.ext import ndb

Expand All @@ -36,6 +34,7 @@ class ExpCopiesRealtimeModel(
jobs.BaseRealtimeDatastoreClassForContinuousComputations):
ExpCopy = ndb.JsonProperty(repeated=True)


class ExpCopiesAggregator(jobs.BaseContinuousComputationManager):
"""A continuous-computation job creating 10 published copies of every
existing exploration, with the eid being '[old_eid]copy[copy_number]',
Expand Down Expand Up @@ -122,6 +121,7 @@ def _get_batch_job_manager_class(cls):
def _handle_incoming_event(cls, active_realtime_layer, event_type, *args):
pass


class DeleteExpCopiesMRJobManager(
jobs.BaseMapReduceJobManagerForContinuousComputations):
"""Job that deletes all explorations in category 'Copies'.
Expand Down
20 changes: 9 additions & 11 deletions core/domain/exp_jobs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@
import collections
import datetime

from core import jobs
from core import jobs_registry
from core.domain import config_services
from core.domain import event_services
from core.domain import exp_domain
from core.domain import exp_jobs
from core.domain import exp_services
import feconf
from core import jobs
from core import jobs_registry
from core.domain import rights_manager
from core.platform import models
from core.tests import test_utils
import feconf
(job_models, exp_models,) = models.Registry.import_models([
models.NAMES.job, models.NAMES.exploration])
from core.domain import rights_manager
search_services = models.Registry.import_search_services()
from core.tests import test_utils
import utils


Expand Down Expand Up @@ -173,7 +173,7 @@ def _run_batch_job_once_and_verify_output(
# publish or publicize exploration
if spec['status'] == rights_manager.EXPLORATION_STATUS_PUBLIC:
rights_manager.publish_exploration(self.owner_id, exp_id)
if spec['status'] == rights_manager.EXPLORATION_STATUS_PUBLICIZED:
elif spec['status'] == rights_manager.EXPLORATION_STATUS_PUBLICIZED:
rights_manager.publish_exploration(self.owner_id, exp_id)
rights_manager.publicize_exploration(self.owner_id, exp_id)

Expand All @@ -187,13 +187,11 @@ def _run_batch_job_once_and_verify_output(

last_updated = None
if exploration.last_updated:
last_updated = utils.get_time_in_millisecs(
exploration.last_updated)
last_updated = exploration.last_updated

created_on = None
if exploration.created_on:
created_on = utils.get_time_in_millisecs(
exploration.created_on)
created_on = exploration.created_on

# manually create the expectated summary specifying title,
# category, etc
Expand All @@ -213,7 +211,7 @@ def _run_batch_job_once_and_verify_output(
created_on=created_on,
last_updated=last_updated)

# calling constructor for field that are not required
# calling constructor for fields that are not required
# and have no default value does not work b/c
# unspecified fields will be empty list in
# expected_job_output but will be unspecified in
Expand Down
Loading

0 comments on commit ab9cd67

Please sign in to comment.