Skip to content

Commit

Permalink
Generalised Review System: Common suggestion view (oppia#5419)
Browse files Browse the repository at this point in the history
* backend and frontend changes for common suggestions view

* fixes to modal UI, and backend API

* minor fix

* WIP

* merge bug-fixes PR

* bug fixes

* linting fixes

* minor fixes

* bug fixes

* remove duplicate ng-show

* linting fixes

* fix status not displayed error

* review changes

* linting fix

* fix backend tests

* filter out your own suggestions from suggestions that can be reviewed

* linting fix

* review changes

* minor UI fixes

* linting fixes

* fix backend test error
  • Loading branch information
nithusha21 authored and anmolshkl committed Aug 15, 2018
1 parent f4c686e commit b685c98
Show file tree
Hide file tree
Showing 22 changed files with 654 additions and 29 deletions.
1 change: 1 addition & 0 deletions assets/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"I18N_DASHBOARD_STATS_TOTAL_PLAYS": "Total Plays",
"I18N_DASHBOARD_STATS_TOTAL_SUBSCRIBERS": "Subscribers",
"I18N_DASHBOARD_SUBSCRIBERS": "Subscribers",
"I18N_DASHBOARD_SUGGESTIONS": "Suggestions",
"I18N_DASHBOARD_TABLE_HEADING_EXPLORATION": "Exploration",
"I18N_DASHBOARD_TABLE_HEADING_LAST_UPDATED": "Last Updated",
"I18N_DASHBOARD_TABLE_HEADING_OPEN_THREADS": "Open Threads",
Expand Down
1 change: 1 addition & 0 deletions assets/i18n/qqq.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"I18N_DASHBOARD_STATS_TOTAL_PLAYS": "Text shown on the stats card on the creator dashboard - This is a label for the Total Plays of a creator's explorations.",
"I18N_DASHBOARD_STATS_TOTAL_SUBSCRIBERS": "Text shown on the stats card on the creator dashboard - This is a label for the total subscriber count of a creator.\n{{Identical|Subscriber}}",
"I18N_DASHBOARD_SUBSCRIBERS": "Title shown in a tab header in the user's dashboard page. - Clicking on this tab header shows the list of users who have subscribed to the user.\n{{Identical|Subscriber}}",
"I18N_DASHBOARD_SUGGESTIONS": "Title shown in a tab header in the user's dashboard page. - Clicking on this tab header shows the list of suggestions created by the user, and the suggestions that can be reviewed by the user.",
"I18N_DASHBOARD_TABLE_HEADING_EXPLORATION": "Text shown as table header in list view for explorations on the creator's dashboard - This is the table header for the Explorations column.\n{{Identical|Exploration}}",
"I18N_DASHBOARD_TABLE_HEADING_LAST_UPDATED": "Text shown as table header in list view for explorations on the creator's dashboard - This is the table header for the Last Updated column.\n{{Identical|Last updated}}",
"I18N_DASHBOARD_TABLE_HEADING_OPEN_THREADS": "Text shown as table header in list view for explorations on the creator's dashboard - This is the table header for the Open Threads column.",
Expand Down
49 changes: 49 additions & 0 deletions core/controllers/creator_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@
from core.domain import feedback_services
from core.domain import role_services
from core.domain import subscription_services
from core.domain import suggestion_services
from core.domain import summary_services
from core.domain import user_jobs_continuous
from core.domain import user_services
from core.platform import models
import feconf
import utils

(feedback_models,) = models.Registry.import_models([models.NAMES.feedback])

EXPLORATION_ID_KEY = 'explorationId'
COLLECTION_ID_KEY = 'collectionId'
QUESTION_ID_KEY = 'questionId'
Expand Down Expand Up @@ -112,6 +116,7 @@ class CreatorDashboardPage(base.BaseHandler):

@acl_decorators.can_access_creator_dashboard
def get(self):

self.values.update({
'nav_mode': feconf.NAV_MODE_CREATOR_DASHBOARD,
'allow_yaml_file_upload': feconf.ALLOW_YAML_FILE_UPLOAD,
Expand Down Expand Up @@ -239,13 +244,57 @@ def _round_average_ratings(rating):
creator_dashboard_display_pref = (
user_settings.creator_dashboard_display_pref)

suggestions_created_by_user = suggestion_services.query_suggestions(
[('author_id', self.user_id)])
suggestions_which_can_be_reviewed = (
suggestion_services
.get_all_suggestions_that_can_be_reviewed_by_user(self.user_id))

for s in suggestions_created_by_user:
s.populate_old_value_of_change()

for s in suggestions_which_can_be_reviewed:
s.populate_old_value_of_change()

suggestion_dicts_created_by_user = (
[s.to_dict() for s in suggestions_created_by_user])
suggestion_dicts_which_can_be_reviewed = (
[s.to_dict() for s in suggestions_which_can_be_reviewed])

ids_of_suggestions_created_by_user = (
[s['suggestion_id'] for s in suggestion_dicts_created_by_user])
ids_of_suggestions_which_can_be_reviewed = (
[s['suggestion_id']
for s in suggestion_dicts_which_can_be_reviewed])

if not constants.ENABLE_GENERALIZED_FEEDBACK_THREADS:
ids_of_suggestions_created_by_user = (
['.'.join(t.split('.')[1:])
for t in ids_of_suggestions_created_by_user])
ids_of_suggestions_which_can_be_reviewed = (
['.'.join(t.split('.')[1:])
for t in ids_of_suggestions_which_can_be_reviewed])

threads_linked_to_suggestions_by_user = (
[t.to_dict() for t in feedback_services.get_multiple_threads(
ids_of_suggestions_created_by_user)])
threads_linked_to_suggestions_which_can_be_reviewed = (
[t.to_dict() for t in feedback_services.get_multiple_threads(
ids_of_suggestions_which_can_be_reviewed)])

self.values.update({
'explorations_list': exp_summary_dicts,
'collections_list': collection_summary_dicts,
'dashboard_stats': dashboard_stats,
'last_week_stats': last_week_stats,
'subscribers_list': subscribers_list,
'display_preference': creator_dashboard_display_pref,
'threads_for_created_suggestions_list': (
threads_linked_to_suggestions_by_user),
'threads_for_suggestions_to_review_list': (
threads_linked_to_suggestions_which_can_be_reviewed),
'created_suggestions_list': suggestion_dicts_created_by_user,
'suggestions_to_review_list': suggestion_dicts_which_can_be_reviewed
})
self.render_json(self.values)

Expand Down
1 change: 1 addition & 0 deletions core/controllers/feedback_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ def test_feedback_threads_with_suggestions(self):
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION, self.EXP_ID, 1,
self.user_id, change_cmd, 'sample description', None)

response = self.get_json(
'%s/%s' % (
feconf.FEEDBACK_THREADLIST_URL_PREFIX, self.EXP_ID))
Expand Down
1 change: 0 additions & 1 deletion core/domain/feedback_jobs_continuous_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ def test_thread_closed_status_changed(self):
'num_open_threads': 1,
'num_total_threads': 1,
})

# Stop job.
ModifiedFeedbackAnalyticsAggregator.stop_computation(user_id)
self.assertEqual(
Expand Down
19 changes: 19 additions & 0 deletions core/domain/feedback_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,25 @@ def get_suggestion(thread_id):
return _get_suggestion_from_model(model) if model else None


def get_multiple_threads(thread_ids):
"""Gets multiple feedback threads.
Args:
thread_ids: list(str). The list of thread IDs.
Returns:
list(FeedbackThread). The list of feedback threads.
"""
if constants.ENABLE_GENERALIZED_FEEDBACK_THREADS:
return [_get_thread_from_model(t)
for t in feedback_models.GeneralFeedbackThreadModel.get_multi(
thread_ids)]
else:
return [_get_thread_from_model(t)
for t in feedback_models.FeedbackThreadModel.get_multi(
thread_ids)]


def _get_thread_from_model(thread_model):
"""Converts the given FeedbackThreadModel to a FeedbackThread object.
Expand Down
2 changes: 1 addition & 1 deletion core/domain/feedback_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def test_get_all_threads(self):
'exploration', self.EXP_ID_1, False)
self.assertEqual(2, len(threads))
self.assertDictContainsSubset(
self.EXPECTED_THREAD_DICT_VIEWER, threads[1].to_dict())
self.EXPECTED_THREAD_DICT_VIEWER, threads[0].to_dict())

def test_get_total_open_threads_before_job_run(self):
self.assertEqual(feedback_services.get_total_open_threads(
Expand Down
18 changes: 18 additions & 0 deletions core/domain/suggestion_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,12 @@ def pre_accept_validate(self):
'Subclasses of BaseSuggestion should implement '
'pre_accept_validate.')

def populate_old_value_of_change(self):
"""Populates the old_value field of the change."""
raise NotImplementedError(
'Subclasses of BaseSuggestion should implement '
'populate_old_value_of_change.')

@property
def is_handled(self):
"""Returns if the suggestion has either been accepted or rejected.
Expand Down Expand Up @@ -307,6 +313,14 @@ def get_change_list_for_accepting_suggestion(self):

return [change]

def populate_old_value_of_change(self):
"""Populates old value of the change."""
exploration = exp_services.get_exploration_by_id(self.target_id)
old_content = (
exploration.states[self.change.state_name].content.to_dict())

self.change.old_value = old_content

def accept(self, commit_message):
"""Accepts the suggestion.
Expand Down Expand Up @@ -451,6 +465,10 @@ def accept(self, unused_commit_message):
question_services.create_new_question_skill_link(
question_dict['id'], self.change.skill_id)

def populate_old_value_of_change(self):
"""Populates old value of the change."""
pass


SUGGESTION_TYPES_TO_DOMAIN_CLASSES = {
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT: (
Expand Down
26 changes: 25 additions & 1 deletion core/domain/suggestion_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ def reject_suggestion(suggestion, reviewer_id, review_message):
mark_review_completed(
suggestion, suggestion_models.STATUS_REJECTED, reviewer_id)


thread_id = suggestion.suggestion_id
if not constants.ENABLE_GENERALIZED_FEEDBACK_THREADS:
thread_id = thread_id[thread_id.find('.') + 1:]
Expand All @@ -285,6 +284,31 @@ def reject_suggestion(suggestion, reviewer_id, review_message):
None, review_message)


def get_all_suggestions_that_can_be_reviewed_by_user(user_id):
"""Returns a list of suggestions which need to be reviewed, in categories
where the user has crossed the minimum score to review.
Args:
user_id: str. The ID of the user.
Returns:
list(Suggestion). A list of suggestions which the given user is allowed
to review.
"""
score_categories = (
user_models.UserContributionScoringModel
.get_all_categories_where_user_can_review(user_id))

if len(score_categories) == 0:
return []

return (
[get_suggestion_from_model(s)
for s in suggestion_models.GeneralSuggestionModel
.get_in_review_suggestions_in_score_categories(
score_categories, user_id)])


def get_user_contribution_scoring_from_model(userContributionScoringModel):
"""Returns the UserContributionScoring domain object corresponding to the
UserContributionScoringModel
Expand Down
40 changes: 40 additions & 0 deletions core/domain/suggestion_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,46 @@ def test_query_suggestions(self):
Exception, 'Not allowed to query on field invalid_field'):
suggestion_services.query_suggestions(queries)

def test_query_suggestions_that_can_be_reviewed_by_user(self):
suggestion_services.create_new_user_contribution_scoring_model(
'user1', 'category1', 15)
suggestion_services.create_new_user_contribution_scoring_model(
'user1', 'category2', 15)
suggestion_services.create_new_user_contribution_scoring_model(
'user1', 'category3', 5)
suggestion_models.GeneralSuggestionModel.create(
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION,
'exp1', 1, suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change, 'category1', 'exploration.exp1.thread_1')
suggestion_models.GeneralSuggestionModel.create(
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION, 'exp1', 1,
suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change, 'category2', 'exploration.exp1.thread_2')
suggestion_models.GeneralSuggestionModel.create(
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION, 'exp1', 1,
suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change, 'category3', 'exploration.exp1.thread_3')
suggestion_models.GeneralSuggestionModel.create(
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION, 'exp1', 1,
suggestion_models.STATUS_REJECTED, 'author_3',
'reviewer_2', self.change, 'category1', 'exploration.exp1.thread_4')
suggestion_models.GeneralSuggestionModel.create(
suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT,
suggestion_models.TARGET_TYPE_EXPLORATION, 'exp1', 1,
suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change, 'category2', 'exploration.exp1.thread_5')
self.assertEqual(len(
suggestion_services
.get_all_suggestions_that_can_be_reviewed_by_user('user1')), 3)
self.assertEqual(len(
suggestion_services
.get_all_suggestions_that_can_be_reviewed_by_user('user2')), 0)



class SuggestionIntegrationTests(test_utils.GenericTestBase):

Expand Down
2 changes: 1 addition & 1 deletion core/storage/feedback/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def get_threads(
the entity. Doesn't include deleted entries.
"""
return cls.get_all().filter(cls.entity_type == entity_type).filter(
cls.entity_id == entity_id).order(cls.last_updated).fetch(limit)
cls.entity_id == entity_id).order(-cls.last_updated).fetch(limit)


class GeneralFeedbackMessageModel(base_models.BaseModel):
Expand Down
25 changes: 25 additions & 0 deletions core/storage/suggestion/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,31 @@ def get_all_stale_suggestions(cls):
return cls.get_all().filter(cls.status == STATUS_IN_REVIEW).filter(
cls.last_updated < threshold_time).fetch()

@classmethod
def get_in_review_suggestions_in_score_categories(
cls, score_categories, user_id):
"""Gets all suggestions which are in review in the given
score_categories.
Args:
score_categories: list(str). list of score categories to query for.
user_id: list(str). The id of the user trying to make this query.
As a user cannot review their own suggestions, suggestions
authored by the user will be excluded.
Returns:
list(SuggestionModel). A list of suggestions that are in the given
score categories, which are in review, but not created by the
given user.
"""
if len(score_categories) == 0:
raise Exception('Received empty list of score categories')

return cls.get_all().filter(cls.status == STATUS_IN_REVIEW).filter(
cls.score_category.IN(score_categories)).filter(
cls.author_id != user_id).fetch(
feconf.DEFAULT_QUERY_LIMIT)

@classmethod
def get_all_score_categories(cls):
"""Gets all the score categories for which suggestions have been
Expand Down
Loading

0 comments on commit b685c98

Please sign in to comment.