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 #19864 : Added feature to generate dummy suggestion questions #20248

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
2 changes: 0 additions & 2 deletions core/controllers/acl_decorators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2482,8 +2482,6 @@ class CanManageContributorsRoleDecoratorTests(test_utils.GenericTestBase):

username = 'user'
user_email = 'user@example.com'
QUESTION_ADMIN_EMAIL: Final = 'questionExpert@app.com'
QUESTION_ADMIN_USERNAME: Final = 'questionExpert'
TRANSLATION_ADMIN_EMAIL: Final = 'translatorExpert@app.com'
TRANSLATION_ADMIN_USERNAME: Final = 'translationExpert'

Expand Down
154 changes: 154 additions & 0 deletions core/controllers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from core.domain import story_services
from core.domain import subtopic_page_domain
from core.domain import subtopic_page_services
from core.domain import suggestion_services
from core.domain import topic_domain
from core.domain import topic_fetchers
from core.domain import topic_services
Expand Down Expand Up @@ -223,6 +224,8 @@ class AdminHandlerNormalizePayloadDict(TypedDict):
collection_id: Optional[str]
num_dummy_exps_to_generate: Optional[int]
num_dummy_exps_to_publish: Optional[int]
num_dummy_question_suggestions_generate: Optional[int]
skill_id: Optional[str]
num_dummy_translation_opportunities_to_generate: Optional[int]
data: Optional[str]
topic_id: Optional[str]
Expand Down Expand Up @@ -256,6 +259,7 @@ class AdminHandler(
'generate_dummy_new_skill_data',
'generate_dummy_blog_post',
'generate_dummy_classroom',
'generate_dummy_question_suggestions',
'upload_topic_similarities',
'regenerate_topic_related_opportunities',
'update_platform_parameter_rules',
Expand Down Expand Up @@ -285,6 +289,18 @@ class AdminHandler(
},
'default_value': None
},
'skill_id': {
'schema': {
'type': 'basestring'
},
'default_value': None
},
'num_dummy_question_suggestions_generate': {
'schema': {
'type': 'int'
},
'default_value': None
},
'num_dummy_exps_to_publish': {
'schema': {
'type': 'int'
Expand Down Expand Up @@ -363,6 +379,9 @@ class AdminHandler(
@acl_decorators.can_access_admin_page
def get(self) -> None:
"""Populates the data on the admin page."""
skill_summaries = skill_services.get_all_skill_summaries()
skill_summary_dicts = [
summary.to_dict() for summary in skill_summaries]
demo_exploration_ids = list(feconf.DEMO_EXPLORATIONS.keys())

topic_summaries = topic_fetchers.get_all_topic_summaries()
Expand Down Expand Up @@ -394,6 +413,7 @@ def get(self) -> None:
'role_to_actions': role_services.get_role_actions(),
'topic_summaries': topic_summary_dicts,
'platform_params_dicts': platform_params_dicts,
'skill_list': skill_summary_dicts,
})

@acl_decorators.can_access_admin_page
Expand Down Expand Up @@ -428,6 +448,10 @@ def post(self) -> None:
Exception. The commit_message must be provided when the action
is update_platform_parameter_rules.
InvalidInputException. The input provided is not valid.
Exception. The skill_id must be provided when
the action is generate_dummy_question_suggestions.
Exception. The num_dummy_question_suggestions_generate must be
provided when the action is generate_dummy_question_suggestions.
"""
assert self.user_id is not None
assert self.normalized_payload is not None
Expand Down Expand Up @@ -505,6 +529,25 @@ def post(self) -> None:
self._generate_dummy_skill_and_questions()
elif action == 'generate_dummy_classroom':
self._generate_dummy_classroom()
elif action == 'generate_dummy_question_suggestions':
skill_id = self.normalized_payload.get('skill_id')
if skill_id is None:
raise Exception(
'The \'skill_id\' must be provided when'
' the action is _generate_dummy_question_suggestions.'
)
num_dummy_question_suggestions_generate = (
self.normalized_payload.get(
'num_dummy_question_suggestions_generate')
)
if num_dummy_question_suggestions_generate is None:
raise Exception(
'The \'num_dummy_question_suggestions_generate\' must'
' be provided when the action is '
'_generate_dummy_question_suggestions.'
)
self._generate_dummy_question_suggestions(
skill_id, num_dummy_question_suggestions_generate)
elif action == 'upload_topic_similarities':
data = self.normalized_payload.get('data')
if data is None:
Expand Down Expand Up @@ -1541,6 +1584,117 @@ def _generate_dummy_classroom(self) -> None:
else:
raise Exception('Cannot generate dummy classroom in production.')

def _generate_dummy_question_suggestions(
self, skill_id: str,
num_dummy_question_suggestions_generate: int) -> None:
"""Generates and loads the database with a specified number of
suggestion question for the selected skill.

Raises:
Exception. Cannot load suggestion questions in production mode.
Exception. User does not have enough rights to generate data.
"""
assert self.user_id is not None
if constants.DEV_MODE:
Copy link
Contributor

Choose a reason for hiding this comment

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

It's usually a very well serving pattern - handle corner cases first and common cases later.

Here

if not constants.DEV_MODE:
  raise...
<main_code>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikitaevg In the whole file it is written like this so I thought it is the appropriate method, so now do I need to change mine code as well as other part of file?

if ((feconf.ROLE_ID_QUESTION_ADMIN not in self.user.roles)
and (not user_services.can_submit_question_suggestions(
self.user_id))):
raise Exception((
'User \'%s\' must be a question submitter or question admin'
' in order to generate question suggestions.'
) % self.username)
for _ in range(num_dummy_question_suggestions_generate):
content_id_generator = translation_domain.ContentIdGenerator()
content_id_generator.generate(
translation_domain.ContentType.CONTENT)
content_id_generator.generate(
translation_domain.ContentType.DEFAULT_OUTCOME)
state = state_domain.State.create_default_state(
'default_state',
content_id_generator.generate(
translation_domain.ContentType.CONTENT),
content_id_generator.generate(
translation_domain.ContentType.DEFAULT_OUTCOME),
is_initial_state=True)
state.update_interaction_id('TextInput')
solution_dict: state_domain.SolutionDict = {
'answer_is_exclusive': False,
'correct_answer': 'Solution',
'explanation': {
'content_id': content_id_generator.generate(
translation_domain.ContentType.SOLUTION),
'html': '<p>This is a solution.</p>',
},
}
hints_list = [
state_domain.Hint(
state_domain.SubtitledHtml(
content_id_generator.generate(
translation_domain.ContentType.HINT),
'<p>This is a hint.</p>')),
]
# Ruling out None for mypy type checking,
# as interaction_id is already updated.
assert state.interaction.id is not None
solution = state_domain.Solution.from_dict(
state.interaction.id, solution_dict)
state.update_interaction_solution(solution)
state.update_interaction_hints(hints_list)
state.update_interaction_customization_args({
'placeholder': {
'value': {
'content_id': content_id_generator.generate(
translation_domain.ContentType.CUSTOMIZATION_ARG, # pylint: disable=line-too-long
extra_prefix='placeholder'),
'unicode_str': 'Enter text here',
},
},
'rows': {'value': 1},
'catchMisspellings': {'value': False}
})
# Here, state is a State domain object and it is created using
# 'create_default_state' method. So, 'state' is a default_state
# and it is always going to contain a default_outcome. Thus to
# narrow down the type from Optional[Outcome] to Outcome for
# default_outcome, we used assert here.
assert state.interaction.default_outcome is not None
state.interaction.default_outcome.labelled_as_correct = True
state.interaction.default_outcome.dest = None
suggestion_change: Dict[
str, Union[str, float, question_domain.QuestionDict]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use AcceptableChangeDictTypes instead of the Union?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikitaevg Yeah, I had a doubt about this. If we change Union[str, float, question_domain.QuestionDict] to change_domain.AcceptableChangeDictTypes, how will we know the exact data type we are receiving? Using change_domain.AcceptableChangeDictTypes doesn't make the code very generic. Currently, we know that we will only receive str, float, or question_domain.QuestionDict, but using AcceptableChangeDictTypes will not define that. Am I missing something about it?

] = {
'cmd': (
question_domain
.CMD_CREATE_NEW_FULLY_SPECIFIED_QUESTION),
'question_dict': {
'id': '',
'version': 0,
'question_state_data': state.to_dict(),
'language_code': 'en',
'question_state_data_schema_version': 1,
'linked_skill_ids': [skill_id],
'inapplicable_skill_misconception_ids': [],
'next_content_id_index': (
content_id_generator.next_content_id_index)
},
'skill_id': skill_id,
'skill_difficulty': 0.3
}

suggestion = suggestion_services.create_suggestion(
feconf.SUGGESTION_TYPE_ADD_QUESTION,
feconf.ENTITY_TYPE_SKILL,
skill_id, 1,
self.user_id, suggestion_change, 'test description')

(
suggestion_services
.update_question_contribution_stats_at_submission(
suggestion))
Comment on lines +1690 to +1693
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you add parenthesis around this expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added parenthesis because this was exceeding max line character limit, so to distribute it into 2 lines.

else:
raise Exception(
'Cannot generate dummy question suggestion in production.')


class AdminRoleHandlerNormalizedGetRequestDict(TypedDict):
"""Dict representation of AdminRoleHandler's GET normalized_request
Expand Down
137 changes: 137 additions & 0 deletions core/controllers/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
from core.domain import story_domain
from core.domain import story_fetchers
from core.domain import story_services
from core.domain import suggestion_services
from core.domain import topic_domain
from core.domain import topic_fetchers
from core.domain import topic_services
Expand Down Expand Up @@ -235,6 +236,48 @@ def test_without_num_dummy_exps_to_publish_action_is_not_performed(

self.logout()

def test_without_skill_id_dummy_question_suggestions_action_is_not_performed( # pylint: disable=line-too-long
self
) -> None:
self.login(self.CURRICULUM_ADMIN_EMAIL, is_super_admin=True)
csrf_token = self.get_new_csrf_token()

assert_raises_regexp_context_manager = self.assertRaisesRegex(
Exception,
'The \'skill_id\' must be provided when the '
'action is _generate_dummy_question_suggestions.'
)
with assert_raises_regexp_context_manager, self.prod_mode_swap:
self.post_json(
'/adminhandler', {
'action': 'generate_dummy_question_suggestions',
'skill_id': None,
'num_dummy_question_suggestions_generate': None
}, csrf_token=csrf_token)

self.logout()

def test_without_num_dummy_question_suggestions_generate_action_is_not_performed( # pylint: disable=line-too-long
self
) -> None:
self.login(self.CURRICULUM_ADMIN_EMAIL, is_super_admin=True)
csrf_token = self.get_new_csrf_token()

assert_raises_regexp_context_manager = self.assertRaisesRegex(
Exception,
'The \'num_dummy_question_suggestions_generate\' must be provided'
' when the action is _generate_dummy_question_suggestions.'
)
with assert_raises_regexp_context_manager, self.prod_mode_swap:
self.post_json(
'/adminhandler', {
'action': 'generate_dummy_question_suggestions',
'skill_id': 'N8daS2n2aoQr',
'num_dummy_question_suggestions_generate': None
}, csrf_token=csrf_token)

self.logout()

def test_without_data_action_upload_topic_similarities_is_not_performed(
self
) -> None:
Expand Down Expand Up @@ -1360,6 +1403,100 @@ def test_cannot_generate_dummy_explorations_in_prod_mode(self) -> None:
self.logout()


class GenerateDummyQuestionSuggestionsTest(test_utils.GenericTestBase):
"""Test the conditions for generation of dummy question suggestions."""

def setUp(self) -> None:
super().setUp()
self.signup(
self.QUESTION_REVIEWER_EMAIL, self.QUESTION_REVIEWER_USERNAME)
self.signup(
self.QUESTION_ADMIN_EMAIL,
self.QUESTION_ADMIN_USERNAME,
is_super_admin=True)
self.question_reviewer_id = self.get_user_id_from_email(
self.QUESTION_REVIEWER_EMAIL)
self.add_user_role(
self.QUESTION_ADMIN_USERNAME,
feconf.ROLE_ID_QUESTION_ADMIN)

def test_generate_dummy_question_suggestions_(self) -> None:
self.login(self.QUESTION_ADMIN_EMAIL, is_super_admin=True)
csrf_token = self.get_new_csrf_token()
self.post_json(
'/contributionrightshandler/submit_question', {
'username': 'questionExpert'
}, csrf_token=csrf_token)

self.post_json(
'/adminhandler', {
'action': 'generate_dummy_question_suggestions',
'skill_id': 'N8daS2n2aoQr',
'num_dummy_question_suggestions_generate': 12
}, csrf_token=csrf_token)

generated_question_suggestions = suggestion_services.get_submitted_suggestions( # pylint: disable=line-too-long
self.get_user_id_from_email(
self.QUESTION_ADMIN_EMAIL),
feconf.SUGGESTION_TYPE_ADD_QUESTION)
self.assertEqual(len(generated_question_suggestions), 12)
self.logout()

def test_cannot_generate_dummy_question_suggestions_in_prod_mode_(# pylint: disable=line-too-long
self) -> None:
self.login(self.QUESTION_ADMIN_EMAIL, is_super_admin=True)
csrf_token = self.get_new_csrf_token()

prod_mode_swap = self.swap(constants, 'DEV_MODE', False)
assert_raises_regexp_context_manager = self.assertRaisesRegex(
Exception, 'Cannot generate dummy question suggestion in production.')

self.post_json(
'/contributionrightshandler/submit_question', {
'username': 'questionExpert'
}, csrf_token=csrf_token)

with assert_raises_regexp_context_manager, prod_mode_swap:
self.post_json(
'/adminhandler', {
'action': 'generate_dummy_question_suggestions',
'skill_id': 'N8daS2n2aoQr',
'num_dummy_question_suggestions_generate': 12
}, csrf_token=csrf_token)

generated_question_suggestions = suggestion_services.get_submitted_suggestions( # pylint: disable=line-too-long
self.get_user_id_from_email(
self.QUESTION_ADMIN_EMAIL),
feconf.SUGGESTION_TYPE_ADD_QUESTION)
self.assertNotEqual(len(generated_question_suggestions), 12)
self.logout()

def test_raises_error_if_not_question_admin_or_question_submitter_(# pylint: disable=line-too-long
self) -> None:
self.login(
self.QUESTION_REVIEWER_EMAIL, is_super_admin=True)
csrf_token = self.get_new_csrf_token()

assert_raises_regexp = self.assertRaisesRegex(
Exception, 'User \'question\' must be a question submitter or question admin'
' in order to generate question suggestions.')

with assert_raises_regexp:
self.post_json(
'/adminhandler', {
'action': 'generate_dummy_question_suggestions',
'skill_id': 'N8daS2n2aoQr',
'num_dummy_question_suggestions_generate': 12
}, csrf_token=csrf_token)

generated_question_suggestions = suggestion_services.get_submitted_suggestions( # pylint: disable=line-too-long
self.get_user_id_from_email(
self.QUESTION_ADMIN_EMAIL),
feconf.SUGGESTION_TYPE_ADD_QUESTION)
self.assertNotEqual(len(generated_question_suggestions), 12)
self.logout()


class GenerateDummyTranslationOpportunitiesTest(test_utils.GenericTestBase):
"""Checks the conditions for generation of dummy translation
opportunities."""
Expand Down