Skip to content

Commit

Permalink
[Milestone 1.1] Implement review test using question player (oppia#6850)
Browse files Browse the repository at this point in the history
* Add globals ALLOWED_INTERACTIONS_CATEGORIES to constants.js

* Remove from exploration editor

* Remove from skill editor

* Remove from topic editor

* Remove from creator dashboard

* Remove from feconf.py

* fix code style

* Add ALLOWED_QUESTION_INTERACTION_CATEGORIES to constants.js and add checking in state_editor to differentiate two interaction catogories

* Remove  ALLOWED_QUESTION_INTERACTION_CATEGORIES from feconf.py

* Required minor change

* Throw an error when path does not match

* replace .location.pathname with currentUrl

* Minor change as directed

* Replace variable name

* Use isInQuestionMode

* sorted order

* remove comma

* Sorted in order

* Fixed indent

* Fixed merge conflicts

* Added review test

* Fixed linting errors

* Fixed linting errors

* Fixed linting errors

* Fixed linting errors

* Fixed linting errors

* Fixed backend bug and addressed required changes

* Fixed linting errors

* Fixed backend error

* Changed set back to list

* Addressed requested changes

* Fixed problems in test

* Fixed linting errors

* Fixed linting errors

* Fixed linting errors

* Fixed backend test error

* Addressed all required changes

* Fixed linting errors

* Fixed linting errors

* Fixed linting errors

* Changes based on comments

* Fixed linting errors

* Made change to review test engine service

* Revert package-lock.json

* Added an exception and fixed a few typos

* Fixed linting errors

* Added comment

* Revert package-lock.json

* Fixed conflict

* Temporary commit package-lock.json

* Fixed errors

* Revert package-lock.json

* Fixed get latest completed node

* linting errors

* Fix typo

* Fix typo

* Changed order

* Fixed merge conflicts

* import review test constants

* Use component directives

* linting

* lint

* controller -> directive

* fix error

* lint

* lint

* frontend error
  • Loading branch information
sophiewu6 authored and vinitamurthi committed Jun 20, 2019
1 parent adde436 commit 12f565e
Show file tree
Hide file tree
Showing 31 changed files with 1,551 additions and 78 deletions.
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,14 @@
# Question project.
/core/controllers/practice_sessions*.py @vinitamurthi
/core/controllers/question*.py @aks681
/core/controllers/review_tests*.py @sophiewu6
/core/domain/question*.py @aks681
/core/storage/question/ @aks681
/core/templates/dev/head/components/entity-creation-services/question-creation.service.ts @aks681
/core/templates/dev/head/components/score-ring/ @vinitamurthi
/core/templates/dev/head/domain/question/ @aks681
/core/templates/dev/head/pages/practice-session-page/ @vinitamurthi
/core/templates/dev/head/pages/review-test-page/ @sophiewu6
/core/templates/dev/head/components/question-directives/question-editor/ @aks681
/core/templates/dev/head/components/question-directives/questions-list/ @aks681
/core/templates/dev/head/components/question-directives/modal-templates/ @aks681
Expand Down
11 changes: 4 additions & 7 deletions core/controllers/reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,6 @@ class QuestionPlayerHandler(base.BaseHandler):
@acl_decorators.open_access
def get(self):
"""Handles GET request."""
start_cursor = self.request.get('start_cursor')
# Skill ids are given as a comma separated list because this is
# a GET request.

Expand All @@ -1030,16 +1029,14 @@ def get(self):
raise self.InvalidInputException(
'Question count has to be greater than 0')

questions, _, next_start_cursor = (
question_services.get_questions_and_skill_descriptions_by_skill_ids(
questions = (
question_services.get_questions_by_skill_ids(
int(question_count),
skill_ids,
start_cursor)
skill_ids)
)

question_dicts = [question.to_dict() for question in questions]
self.values.update({
'question_dicts': question_dicts,
'next_start_cursor': next_start_cursor
'question_dicts': question_dicts
})
self.render_json(self.values)
19 changes: 5 additions & 14 deletions core/controllers/reader_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,23 +320,14 @@ def setUp(self):

def test_questions_are_returned_successfully(self):
# Call the handler.
url = '%s?question_count=%s&skill_ids=%s&start_cursor=' % (
url = '%s?question_count=%s&skill_ids=%s' % (
feconf.QUESTIONS_URL_PREFIX, '1', self.skill_id)
json_response_1 = self.get_json(url)
next_cursor = json_response_1['next_start_cursor']
self.assertEqual(len(json_response_1['question_dicts']), 1)
json_response_2 = self.get_json(
'%s?question_count=%s&skill_ids=%s&start_cursor=%s' % (
feconf.QUESTIONS_URL_PREFIX, '1', self.skill_id,
next_cursor))
self.assertEqual(len(json_response_2['question_dicts']), 1)
self.assertNotEqual(
json_response_1['question_dicts'][0]['id'],
json_response_2['question_dicts'][0]['id'])

def test_question_count_more_than_available_returns_all_questions(self):
# Call the handler.
url = '%s?question_count=%s&skill_ids=%s&start_cursor=' % (
url = '%s?question_count=%s&skill_ids=%s' % (
feconf.QUESTIONS_URL_PREFIX, '5', self.skill_id)
json_response = self.get_json(url)
self.assertEqual(len(json_response['question_dicts']), 2)
Expand All @@ -351,7 +342,7 @@ def test_multiple_skill_id_returns_questions(self):
self._create_valid_question_data('ABC'), [self.skill_id])
question_services.create_new_question_skill_link(
self.editor_id, question_id_3, skill_id_2, 0.5)
url = '%s?question_count=%s&skill_ids=%s,%s&start_cursor=' % (
url = '%s?question_count=%s&skill_ids=%s,%s' % (
feconf.QUESTIONS_URL_PREFIX, '3', self.skill_id, skill_id_2)
json_response = self.get_json(url)
self.assertEqual(len(json_response['question_dicts']), 3)
Expand All @@ -361,14 +352,14 @@ def test_multiple_skill_id_returns_questions(self):

def test_invalid_skill_id_returns_no_questions(self):
# Call the handler.
url = '%s?question_count=%s&skill_ids=%s&start_cursor=' % (
url = '%s?question_count=%s&skill_ids=%s' % (
feconf.QUESTIONS_URL_PREFIX, '1', 'invalid_skill_id')
json_response = self.get_json(url)
self.assertEqual(len(json_response['question_dicts']), 0)

def test_question_count_zero_raises_invalid_input_exception(self):
# Call the handler.
url = '%s?question_count=%s&skill_ids=%s&start_cursor=' % (
url = '%s?question_count=%s&skill_ids=%s' % (
feconf.QUESTIONS_URL_PREFIX, '0', self.skill_id)
self.get_json(url, expected_status_int=400)

Expand Down
100 changes: 100 additions & 0 deletions core/controllers/review_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# Copyright 2019 The Oppia Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS-IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Controllers for the review tests page."""

from constants import constants
from core.controllers import acl_decorators
from core.controllers import base
from core.domain import dependency_registry
from core.domain import interaction_registry
from core.domain import obj_services
from core.domain import story_services
import feconf
import jinja2


class ReviewTestsPage(base.BaseHandler):
"""Renders the review tests page."""

@acl_decorators.can_access_story_viewer_page
def get(self, story_id):
"""Handles GET requests."""

if not constants.ENABLE_NEW_STRUCTURE_PLAYERS:
raise self.PageNotFoundException

story = story_services.get_story_by_id(story_id)

if story is None:
raise self.PageNotFoundException(
Exception('The story with the given id doesn\'t exist.'))

interaction_ids = feconf.ALLOWED_QUESTION_INTERACTION_IDS

interaction_dependency_ids = (
interaction_registry.Registry.get_deduplicated_dependency_ids(
interaction_ids))
dependencies_html, additional_angular_modules = (
dependency_registry.Registry.get_deps_html_and_angular_modules(
interaction_dependency_ids))

interaction_templates = (
interaction_registry.Registry.get_interaction_html(
interaction_ids))

self.values.update({
'DEFAULT_OBJECT_VALUES': obj_services.get_default_object_values(),
'additional_angular_modules': additional_angular_modules,
'INTERACTION_SPECS': interaction_registry.Registry.get_all_specs(),
'interaction_templates': jinja2.utils.Markup(
interaction_templates),
'dependencies_html': jinja2.utils.Markup(dependencies_html),
'story_name': story.title
})
self.render_template('dist/review-test-page.mainpage.html')


class ReviewTestsPageDataHandler(base.BaseHandler):
"""Fetches relevant data for the review tests page. This handler should
be called only if the user has completed at least one exploration in
the story.
"""
GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON

@acl_decorators.can_access_story_viewer_page
def get(self, story_id):
"""Handles GET requests."""
if not constants.ENABLE_NEW_STRUCTURE_PLAYERS:
raise self.PageNotFoundException

story = story_services.get_story_by_id(story_id)
latest_completed_node_ids = (
story_services.get_latest_completed_node_ids(self.user_id, story_id)
)

if story is None:
raise self.PageNotFoundException(
Exception('The story with the given id doesn\'t exist.'))
if len(latest_completed_node_ids) == 0:
raise self.PageNotFoundException

acquired_skill_ids = story.get_acquired_skill_ids_for_node_ids(
latest_completed_node_ids
)

self.values.update({
'skill_ids': acquired_skill_ids
})
self.render_json(self.values)
160 changes: 160 additions & 0 deletions core/controllers/review_tests_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
# Copyright 2019 The Oppia Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS-IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Tests for the review tests page."""

from constants import constants
from core.domain import story_domain
from core.domain import story_services
from core.domain import user_services
from core.tests import test_utils
import feconf


class BaseReviewTestsControllerTests(test_utils.GenericTestBase):

OWNER_EMAIL = '[email protected]'
VIEWER_EMAIL = '[email protected]'
VIEWER_USERNAME = 'viewer'

def setUp(self):
"""Completes the sign-up process for the various users."""
super(BaseReviewTestsControllerTests, self).setUp()
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
self.signup(self.VIEWER_EMAIL, self.VIEWER_USERNAME)

self.admin_id = self.get_user_id_from_email(self.ADMIN_EMAIL)
self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
self.viewer_id = self.get_user_id_from_email(self.VIEWER_EMAIL)

self.set_admins([self.ADMIN_USERNAME])
self.admin = user_services.UserActionsInfo(self.admin_id)

self.topic_id = 'topic_id'
self.story_id_1 = 'story_id_1'
self.story_id_2 = 'story_id_2'
self.node_id = 'node_1'
self.node_id_2 = 'node_2'
self.exp_id = 'exp_id'

self.save_new_valid_exploration(self.exp_id, self.owner_id)
self.publish_exploration(self.owner_id, self.exp_id)

self.node_1 = {
'id': self.node_id,
'title': 'Title 1',
'destination_node_ids': [],
'acquired_skill_ids': ['skill_id_1', 'skill_id_2'],
'prerequisite_skill_ids': [],
'outline': '',
'outline_is_finalized': False,
'exploration_id': self.exp_id
}

self.story = story_domain.Story.create_default_story(
self.story_id_1, 'Public Story Title', self.topic_id)
self.story.story_contents.nodes = [
story_domain.StoryNode.from_dict(self.node_1)
]
self.story.story_contents.initial_node_id = self.node_id
self.story.story_contents.next_node_id = self.node_id_2
story_services.save_new_story(self.admin_id, self.story)

self.story_2 = story_domain.Story.create_default_story(
self.story_id_2, 'Private Story Title', self.topic_id)
story_services.save_new_story(self.admin_id, self.story_2)

story_services.publish_story(self.story_id_1, self.admin_id)

self.login(self.VIEWER_EMAIL)


class ReviewTestsPageTests(BaseReviewTestsControllerTests):

def test_get_fails_when_new_structures_not_enabled(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', False):
self.get_html_response(
'%s/%s' % (
feconf.REVIEW_TEST_URL_PREFIX,
self.story_id_1),
expected_status_int=404)

def test_any_user_can_access_review_tests_page(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_html_response(
'%s/%s' % (
feconf.REVIEW_TEST_URL_PREFIX,
self.story_id_1))

def test_no_user_can_access_unpublished_story_review_sessions_page(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_html_response(
'%s/%s' % (
feconf.REVIEW_TEST_URL_PREFIX, self.story_id_2),
expected_status_int=404)

def test_get_fails_when_story_doesnt_exist(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_html_response(
'%s/%s' % (
feconf.REVIEW_TEST_URL_PREFIX,
'story_id_3'),
expected_status_int=404)


class ReviewTestsPageDataHandlerTests(BaseReviewTestsControllerTests):

def test_get_fails_when_new_structures_not_enabled(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', False):
self.get_json(
'%s/%s' % (
feconf.REVIEW_TEST_DATA_URL_PREFIX,
self.story_id_1),
expected_status_int=404)

def test_any_user_can_access_review_tests_data(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
story_services.record_completed_node_in_story_context(
self.viewer_id, self.story_id_1, self.node_id)
json_response = self.get_json(
'%s/%s' % (
feconf.REVIEW_TEST_DATA_URL_PREFIX,
self.story_id_1))
self.assertEqual(len(json_response['skill_ids']), 2)
self.assertEqual(json_response['skill_ids'][0], 'skill_id_1')
self.assertEqual(json_response['skill_ids'][1], 'skill_id_2')

def test_no_user_can_access_unpublished_story_review_sessions_data(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_json(
'%s/%s' % (
feconf.REVIEW_TEST_DATA_URL_PREFIX, self.story_id_2),
expected_status_int=404)

def test_get_fails_when_story_doesnt_exist(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_json(
'%s/%s' % (
feconf.REVIEW_TEST_DATA_URL_PREFIX,
'story_id_3'),
expected_status_int=404)

def test_get_fails_when_no_completed_story_node(self):
with self.swap(constants, 'ENABLE_NEW_STRUCTURE_PLAYERS', True):
self.get_json(
'%s/%s' % (
feconf.REVIEW_TEST_DATA_URL_PREFIX,
self.story_id_1),
expected_status_int=404)
Loading

0 comments on commit 12f565e

Please sign in to comment.