Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix part of #13162: added schemas for SkillMasteryDataHandler #14395

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions core/controllers/domain_objects_validator.py
Expand Up @@ -26,11 +26,31 @@
from core.domain import collection_domain
from core.domain import config_domain
from core.domain import exp_domain
from core.domain import skill_domain
from core.domain import state_domain

from typing import Dict, Optional, Union


def validate_mastery_change(mastery_change_per_skill):
"""Validates user skill mastery changes.

Args:
mastery_change_per_skill: dict. Data that needs to be validated.

Returns:
dict(str, float). Returns mastery_change_per_skill after validation.
"""
for (skill_id, mastery) in mastery_change_per_skill:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (skill_id, mastery) in mastery_change_per_skill:
for skill_id, mastery in mastery_change_per_skill:

skill_domain.Skill.require_valid_skill_id(skill_id)
if not isinstance(mastery, float):
raise Exception(
'degree of mastery change should be a float, received'
': %s' % mastery)

return mastery_change_per_skill


def validate_exploration_change(obj):
"""Validates exploration change.

Expand Down
20 changes: 20 additions & 0 deletions core/controllers/skill_mastery.py
Expand Up @@ -21,6 +21,7 @@
from core import utils
from core.controllers import acl_decorators
from core.controllers import base
from core.controllers import domain_objects_validator
from core.domain import skill_domain
from core.domain import skill_fetchers
from core.domain import skill_services
Expand All @@ -33,6 +34,25 @@ class SkillMasteryDataHandler(base.BaseHandler):
"""

GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
URL_PATH_ARGS_SCHEMAS = {}
HANDLER_ARGS_SCHEMAS = {
'GET': {
'comma_separated_exp_ids': {
'schema': {
'type': 'basestring'
}
}
Comment on lines +40 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor this to use JsonEncodedInString see other places to understand how it works.

},
'PUT': {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vojtechjelinek Shouldn't def put use self.normalized_payload? Also, is there a corresponding normalized_request for get?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, use self.normalized_payload instead of self.payload everywhere.

'mastery_change_per_skill': {
'schema': {
'type': 'object_dict',
'validation_method': (
domain_objects_validator.validate_mastery_change)
}
}
}
}

@acl_decorators.can_access_learner_dashboard
def get(self):
Expand Down
1 change: 0 additions & 1 deletion core/handler_schema_constants.py
Expand Up @@ -80,7 +80,6 @@
'SiteLanguageHandler',
'SkillDataHandler',
'SkillDescriptionHandler',
'SkillMasteryDataHandler',
'SkillsDashboardPageDataHandler',
'StartedTranslationTutorialEventHandler',
'StateCompleteEventHandler',
Expand Down