Skip to content

Commit

Permalink
Fix part of oppia#1366: Unresolved answers part of creator dashboard (o…
Browse files Browse the repository at this point in the history
…ppia#2101)

* Service to get total unresolved answers for exp

* Use get_multi instead of get

* Get unresolved_answers counts exploration-wise

* Revert previous code; Re-write implementation and tests

* Update functionality; write more tests

* Frontend fixes

* Fix minor frontend bugs

* Partially address review comments

* Write tests to handle multiple cases

* Added comment for explorations_states_tuples

* Fix errors with code

* Rename method and remove pylint ignore

* Address review comments

* Fix failing tests
  • Loading branch information
526avijitgupta authored and wxyxinyu committed Aug 8, 2016
1 parent 5dcfae6 commit 26ff6df
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 21 deletions.
27 changes: 19 additions & 8 deletions core/controllers/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from core.domain import exp_domain
from core.domain import exp_services
from core.domain import feedback_services
from core.domain import stats_services
from core.domain import subscription_services
from core.domain import summary_services
from core.domain import user_jobs_continuous
Expand Down Expand Up @@ -167,19 +168,29 @@ def _get_intro_card_color(category):
subscription_services.get_collection_ids_subscribed_to(
self.user_id))))

explorations_list = summary_services.get_displayable_exp_summary_dicts(
exp_summary_list = summary_services.get_displayable_exp_summary_dicts(
subscribed_exploration_summaries)
collections_list = []
collection_summary_list = []

feedback_thread_analytics = (
feedback_services.get_thread_analytics_multi(
exploration_ids_subscribed_to))

for ind, exploration in enumerate(explorations_list):
unresolved_answers_dict = (
stats_services.get_exps_unresolved_answers_count_for_default_rule(
exploration_ids_subscribed_to))

for ind, exploration in enumerate(exp_summary_list):
exploration.update(feedback_thread_analytics[ind].to_dict())
exploration.update({
'num_unresolved_answers': (
unresolved_answers_dict[exploration['id']]
if exploration['id'] in unresolved_answers_dict else 0
)
})

explorations_list = sorted(
explorations_list,
exp_summary_list = sorted(
exp_summary_list,
key=lambda x: (x['num_open_threads'], x['last_updated_msec']),
reverse=True)

Expand All @@ -188,7 +199,7 @@ def _get_intro_card_color(category):
for collection_summary in subscribed_collection_summaries:
# TODO(sll): Reuse _get_displayable_collection_summary_dicts()
# in summary_services, instead of replicating it like this.
collections_list.append({
collection_summary_list.append({
'id': collection_summary.id,
'title': collection_summary.title,
'category': collection_summary.category,
Expand All @@ -208,8 +219,8 @@ def _get_intro_card_color(category):
})

self.values.update({
'explorations_list': explorations_list,
'collections_list': collections_list,
'explorations_list': exp_summary_list,
'collections_list': collection_summary_list,
'dashboard_stats': user_services.get_user_dashboard_stats(
self.user_id)
})
Expand Down
41 changes: 41 additions & 0 deletions core/domain/stats_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,47 @@ def get_top_unresolved_answers_for_default_rule(exploration_id, state_name):
}


def get_exps_unresolved_answers_count_for_default_rule(exp_ids):
"""Gets answer counts per exploration for the answer groups for default
rule across all states for explorations with ids in exp_ids.
Note that this method currently returns the counts only for the DEFAULT
rule. This should ideally handle all types of unresolved answers.
Returns:
A dict, keyed by the string '{exp_id}', whose values are the number of
unresolved answers that exploration has.
"""
explorations = exp_services.get_multiple_explorations_by_id(exp_ids)

# The variable `exploration_states_tuples` is a list of all
# (exp_id, state_name) tuples for the given exp_ids.
# E.g. - [
# ('eid1', 'Introduction'),
# ('eid1', 'End'),
# ('eid2', 'Introduction'),
# ('eid3', 'Introduction')
# ]
# when exp_ids = ['eid1', 'eid2', 'eid3'].
explorations_states_tuples = [
(exp_domain_object.id, state_key)
for exp_domain_object in explorations.values()
for state_key in exp_domain_object.states
]
exploration_states_answers_list = get_top_state_rule_answers_multi(
explorations_states_tuples, [exp_domain.DEFAULT_RULESPEC_STR])
exps_answers_mapping = {}

for ind, statewise_answers in enumerate(exploration_states_answers_list):
for answer in statewise_answers:
exp_id = explorations_states_tuples[ind][0]
if exp_id not in exps_answers_mapping:
exps_answers_mapping[exp_id] = 0
exps_answers_mapping[exp_id] += answer['count']

return exps_answers_mapping


def get_state_rules_stats(exploration_id, state_name):
"""Gets statistics for the answer groups and rules of this state.
Expand Down
110 changes: 110 additions & 0 deletions core/domain/stats_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,27 @@ class UnresolvedAnswersTests(test_utils.GenericTestBase):
"""Test the unresolved answers methods."""

DEFAULT_RULESPEC_STR = exp_domain.DEFAULT_RULESPEC_STR
CLASSIFIER_RULESPEC_STR = exp_domain.CLASSIFIER_RULESPEC_STR
STATE_2_NAME = 'State 2'

def _create_and_update_fake_exploration(self, exp_id):
exp = exp_domain.Exploration.create_default_exploration(exp_id)
exp_services.save_new_exploration('[email protected]', exp)
exp_services.update_exploration('[email protected]', exp_id, [{
'cmd': 'edit_state_property',
'state_name': exp.init_state_name,
'property_name': 'widget_id',
'new_value': 'TextInput',
}, {
'cmd': 'add_state',
'state_name': self.STATE_2_NAME,
}, {
'cmd': 'edit_state_property',
'state_name': self.STATE_2_NAME,
'property_name': 'widget_id',
'new_value': 'TextInput',
}], 'Add new state')
return exp

def test_get_top_unresolved_answers(self):
self.assertEquals(
Expand All @@ -493,6 +514,95 @@ def test_get_top_unresolved_answers(self):
stats_services.get_top_unresolved_answers_for_default_rule(
'eid', 'sid'), {})

def test_unresolved_answers_count_for_single_exploration(self):
exp_1 = self._create_and_update_fake_exploration('eid1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {})
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.DEFAULT_RULESPEC_STR, 'a1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {'eid1': 1})

def test_unresolved_answers_count_for_multiple_explorations(self):
exp_1 = self._create_and_update_fake_exploration('eid1')
exp_2 = self._create_and_update_fake_exploration('eid2')
exp_3 = self._create_and_update_fake_exploration('eid3')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1', 'eid2', 'eid3']), {})
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.DEFAULT_RULESPEC_STR, 'a1')
event_services.AnswerSubmissionEventHandler.record(
'eid2', 1, exp_2.init_state_name, self.DEFAULT_RULESPEC_STR, 'a3')
event_services.AnswerSubmissionEventHandler.record(
'eid2', 1, exp_2.init_state_name, self.DEFAULT_RULESPEC_STR, 'a2')
event_services.AnswerSubmissionEventHandler.record(
'eid3', 1, exp_3.init_state_name, self.DEFAULT_RULESPEC_STR, 'a2')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1', 'eid2', 'eid3']), {'eid1': 1, 'eid2': 2, 'eid3': 1})

def test_unresolved_answers_count_when_answers_marked_as_resolved(self):
exp_1 = self._create_and_update_fake_exploration('eid1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {})
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.DEFAULT_RULESPEC_STR, 'a1')
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.DEFAULT_RULESPEC_STR, 'a2')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {'eid1': 2})

event_services.DefaultRuleAnswerResolutionEventHandler.record(
'eid1', exp_1.init_state_name, ['a1'])
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {'eid1': 1})

exp_2 = self._create_and_update_fake_exploration('eid2')
event_services.AnswerSubmissionEventHandler.record(
'eid2', 1, exp_2.init_state_name, self.DEFAULT_RULESPEC_STR, 'a1')
event_services.DefaultRuleAnswerResolutionEventHandler.record(
'eid1', exp_1.init_state_name, ['a2'])
event_services.DefaultRuleAnswerResolutionEventHandler.record(
'eid2', exp_1.init_state_name, ['a1'])
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1', 'eid2']), {})

def test_unresolved_answers_count_for_multiple_states(self):
exp_1 = self._create_and_update_fake_exploration('eid1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {})
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.DEFAULT_RULESPEC_STR, 'a1')
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, self.STATE_2_NAME, self.DEFAULT_RULESPEC_STR, 'a1')
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, self.STATE_2_NAME, self.DEFAULT_RULESPEC_STR, 'a2')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {'eid1': 3})

def test_unresolved_answers_count_for_non_default_rules(self):
exp_1 = self._create_and_update_fake_exploration('eid1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {})
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, exp_1.init_state_name, self.CLASSIFIER_RULESPEC_STR, 'a1'
)
event_services.AnswerSubmissionEventHandler.record(
'eid1', 1, self.STATE_2_NAME, self.CLASSIFIER_RULESPEC_STR, 'a1')
self.assertEquals(
stats_services.get_exps_unresolved_answers_count_for_default_rule(
['eid1']), {})


class EventLogEntryTests(test_utils.GenericTestBase):
"""Test for the event log creation."""
Expand Down
2 changes: 1 addition & 1 deletion core/templates/dev/head/dashboard/Dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ oppia.controller('Dashboard', [
$window.location = '/create/' + explorationId;
};

$scope.myExplorationsView = 'list';
$scope.myExplorationsView = 'card';
$scope.setMyExplorationsView = function(viewType) {
$scope.myExplorationsView = viewType;
};
Expand Down
28 changes: 17 additions & 11 deletions core/templates/dev/head/dashboard/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,19 @@ <h1 class="stat-value"><[dashboardStats.total_plays]></h1>
class="oppia-dashboard-list-view-item">
<table class="oppia-dashboard-table">
<colgroup>
<col style="width: 40%;">
<col style="width: 15%;">
<col style="width: 15%;">
<col style="width: 15%;">
<col style="width: 15%;">
<col style="width: 30%;">
<col style="width: 14%;">
<col style="width: 14%;">
<col style="width: 14%;">
<col style="width: 14%;">
<col style="width: 14%;">
</colgroup>
<tr>
<th>Exploration</th>
<th>Rating</th>
<th>Plays</th>
<th>Feedback</th>
<th>Unresolved Answers</th>
<th>Last Updated</th>
</tr>
<tr ng-repeat="exploration in explorationsList track by exploration.id"
Expand All @@ -129,10 +131,10 @@ <h1 class="stat-value"><[dashboardStats.total_plays]></h1>
featured
</span>
</td>
<td colspan="3"
<td colspan="4"
ng-if="exploration.status === 'private'"
class="exp-private-text">
This exploration is private. Publish to receive statistics.
This exploration is private. Publish it to receive statistics.
</td>
<td ng-if="exploration.status !== 'private'">
<[(getAverageRating(exploration.ratings) | number:1) || 'N/A']>
Expand All @@ -143,6 +145,9 @@ <h1 class="stat-value"><[dashboardStats.total_plays]></h1>
<td ng-if="exploration.status !== 'private'">
<[exploration.num_total_threads]>
</td>
<td ng-if="exploration.status !== 'private'">
<[exploration.num_unresolved_answers]>
</td>
<td>
<[getLocaleAbbreviatedDatetimeString(exploration.last_updated_msec)]>
</td>
Expand All @@ -166,7 +171,7 @@ <h2 class="activity-title protractor-test-exp-summary-tile-title">

<div ng-attr-section="'<['right-section']>">
<div ng-if="exploration.status === 'private'" class="exp-private-text">
This exploration is private. Publish to receive statistics.
This exploration is private. Publish it to receive statistics.
</div>
<ul ng-if="exploration.status !== 'private'"
layout="row"
Expand Down Expand Up @@ -222,7 +227,6 @@ <h2 class="activity-title protractor-test-exp-summary-tile-title">
</div>
</md-card>
</div>

</div>

{% if can_create_collections %}
Expand All @@ -237,7 +241,9 @@ <h2 class="activity-title protractor-test-exp-summary-tile-title">
<a ng-href="/collection_editor/create/<[collection.id]>" class="oppia-dashboard-tile-container-link" style="text-decoration: none;">
<div class="oppia-dashboard-tile-metadata-parent hidden-xs">
<div class="oppia-dashboard-tile-metadata">
Created: <span style="color: black;"><[getLocaleAbbreviatedDatetimeString(collection.created_on_msec)]></span>
Created:<span style="color: black;">
<[getLocaleAbbreviatedDatetimeString(collection.created_on)]>
</span>

<br>
<br>
Expand Down Expand Up @@ -282,7 +288,7 @@ <h2 class="activity-title protractor-test-exp-summary-tile-title">
<span style="color: #888">
<strong><[collection.category]></strong>
<span style="font-weight: bold;" ng-if="collection.category">&middot;</span>
Last updated <[getLocaleAbbreviatedDatetimeString(collection.last_updated_msec)]>
Last updated <[getLocaleAbbreviatedDatetimeString(collection.last_updated)]>
</span>

<span ng-if="collection.status === 'publicized'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ describe('Dashboard backend API service', function() {
thumbnail_bg_color: '#bb8b2f',
num_views: 2,
num_open_threads: 0,
num_total_threads: 0
num_total_threads: 0,
num_unresolved_answers: 2
}],
collections_list: [],
dashboard_stats: {
Expand Down
1 change: 1 addition & 0 deletions i18n/pt.json
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
"I18N_LIBRARY_GROUPS_SCIENCE" : "Ciências",
"I18N_LIBRARY_GROUPS_SOCIAL_SCIENCE": "Ciências Sociais",
"I18N_LIBRARY_GROUPS_TOP_RATED_EXPLORATIONS": "Explorações bem Classificadas",
"I18N_LIBRARY_LAST_UPDATED": "Ultima atualização",
"I18N_LIBRARY_LOADING": "A carregar",
"I18N_LIBRARY_MAIN_HEADER": "Imagina o que poderás aprender hoje...",
"I18N_LIBRARY_N/A": "N/D",
Expand Down

0 comments on commit 26ff6df

Please sign in to comment.