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.

Sign up for GitHub

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 #16069: Add a "Newest First" tag for reviewable questions #16534

Merged
merged 31 commits into from
Jan 10, 2023
Merged

Fix #16069: Add a "Newest First" tag for reviewable questions #16534

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0366952
Add backend functions for sorting questions with the newest first
qinghaoyang Nov 13, 2022
412fca4
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Nov 13, 2022
5283c47
Fix lint errors
qinghaoyang Nov 13, 2022
45fc067
Completes the covereage test
qinghaoyang Nov 13, 2022
449dc4b
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Nov 29, 2022
748de42
Fix mypy errors
qinghaoyang Nov 29, 2022
9f470bb
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Dec 4, 2022
581281c
Change backend functions
qinghaoyang Dec 6, 2022
baa32ff
Change frontend functions
qinghaoyang Dec 6, 2022
8370912
Fix lint errors
qinghaoyang Dec 6, 2022
c33e9d3
Fix mypy errors
qinghaoyang Dec 6, 2022
304673a
Fix arguments error
qinghaoyang Dec 6, 2022
5734746
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Dec 22, 2022
75bc4d8
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Dec 29, 2022
6c2bfdd
Use positional args
qinghaoyang Dec 29, 2022
807fdc8
Use Date as the default choice
qinghaoyang Dec 29, 2022
e04ce60
Fix backend errors
qinghaoyang Dec 29, 2022
8cc15ce
Add a constant in constants.ts
qinghaoyang Dec 29, 2022
19371f4
Fetch a batch of items one time
qinghaoyang Dec 30, 2022
44c8d20
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Dec 30, 2022
0959288
Fix mypy errors
qinghaoyang Dec 30, 2022
b7848b1
Update frontend test
qinghaoyang Dec 30, 2022
91f889c
Fixed frontend bugs
qinghaoyang Dec 31, 2022
39de584
Set sort_key as a mandatory argument
qinghaoyang Jan 9, 2023
c3fc3b4
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Jan 9, 2023
3f3e83e
Update frontend tests
qinghaoyang Jan 9, 2023
766a329
Change the default sort key of reviewable translations to date
qinghaoyang Jan 10, 2023
8d9849a
Fix a bug about calculating offset
qinghaoyang Jan 10, 2023
4e80671
Fix mypy errors
qinghaoyang Jan 10, 2023
428f2ee
Add indexes
qinghaoyang Jan 10, 2023
a7cadb5
Merge remote-tracking branch 'upstream/develop' into sorting_questions
qinghaoyang Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Change the default sort key of reviewable translations to date
  • Loading branch information
@qinghaoyang
qinghaoyang committed Jan 10, 2023
commit 766a329bfa36a2b9fb4dde0a90db036b5ef64e1f
1 change: 0 additions & 1 deletion assets/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5858,7 +5858,6 @@ export default {
],

"SUGGESTIONS_SORT_KEY_DATE": "Date",
"SUGGESTIONS_SORT_KEY_NULL": "",

"ACTION_REMOVE_ALL_REVIEW_RIGHTS": "all",
"ACTION_REMOVE_SPECIFIC_CONTRIBUTION_RIGHTS": "specific",
Expand Down
14 changes: 7 additions & 7 deletions core/controllers/suggestion_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2808,7 +2808,7 @@ def test_exploration_handler_returns_data_with_no_exploration_id(
'/getreviewablesuggestions/exploration/translate_content', {
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
})
self.assertEqual(len(response['suggestions']), 0)
self.assertEqual(response['next_offset'], 0)
Expand All @@ -2821,7 +2821,7 @@ def test_exploration_handler_returns_data_with_valid_exploration_id(
'exploration_id': self.EXP_ID,
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
})
self.assertEqual(len(response['suggestions']), 1)
self.assertEqual(response['next_offset'], 1)
Expand Down Expand Up @@ -2909,7 +2909,7 @@ def test_exploration_handler_does_not_return_obsolete_suggestions(
'exploration_id': exp_100.id,
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
})
self.assertEqual(len(response['suggestions']), 1)
self.assertEqual(response['next_offset'], 1)
Expand Down Expand Up @@ -2943,7 +2943,7 @@ def test_exploration_handler_does_not_return_obsolete_suggestions(
'exploration_id': exp_100.id,
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
})
self.assertEqual(len(response['suggestions']), 0)
self.assertEqual(response['next_offset'], 1)
Expand All @@ -2953,7 +2953,7 @@ def test_topic_translate_handler_returns_no_data(self) -> None:
'/getreviewablesuggestions/topic/translate_content', {
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
})
self.assertEqual(response, {})

Expand Down Expand Up @@ -3013,7 +3013,7 @@ def test_handler_with_invalid_suggestion_type_raise_error(self) -> None:
'/getreviewablesuggestions/exploration/invalid_suggestion_type', {
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
},
expected_status_int=404
)
Expand All @@ -3023,7 +3023,7 @@ def test_handler_with_invalid_target_type_raise_error(self) -> None:
'/getreviewablesuggestions/invalid_target_type/translate_content', {
'limit': constants.OPPORTUNITIES_PAGE_SIZE,
'offset': 0,
'sort_key': constants.SUGGESTIONS_SORT_KEY_NULL
'sort_key': constants.SUGGESTIONS_SORT_KEY_DATE
},
expected_status_int=400
)
5 changes: 1 addition & 4 deletions core/feconf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1639,10 +1639,7 @@ def get_empty_ratings() -> Dict[str, int]:
]

# The sort keys of submitted questions shown on the Contributor Dashboard.
SUGGESTIONS_SORT_KEYS = [
constants.SUGGESTIONS_SORT_KEY_DATE,
constants.SUGGESTIONS_SORT_KEY_NULL
]
SUGGESTIONS_SORT_KEYS = [constants.SUGGESTIONS_SORT_KEY_DATE]

# Prefix for all access validation handlers.
# The naming scheme for access validation handlers is
Expand Down
89 changes: 83 additions & 6 deletions core/storage/suggestion/gae_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,7 @@ def get_in_review_translation_suggestions_by_offset(
limit: Optional[int],
offset: int,
user_id: str,
# Currently, the argument sort_key is not used in this function, but it
# will be used when new sorting options are added in the frontend.
sort_key: Optional[str], # pylint: disable=unused-argument
sort_key: Optional[str],
language_codes: List[str]
) -> Tuple[Sequence[GeneralSuggestionModel], int]:
"""Fetches translation suggestions that are in-review where the
Expand All @@ -514,6 +512,46 @@ def get_in_review_translation_suggestions_by_offset(
next_offset: int. The input offset + the number of results
returned by the current query.
"""
if sort_key == constants.SUGGESTIONS_SORT_KEY_DATE:
# The first sort property must be the same as the property to which
# an inequality filter is applied. Thus, the inequality filter on
# author_id can not be used here.
suggestion_query = cls.get_all().filter(datastore_services.all_of(
cls.status == STATUS_IN_REVIEW,
cls.suggestion_type == feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
cls.language_code.IN(language_codes)
)).order(-cls.created_on)

sorted_results: List[GeneralSuggestionModel] = []

if limit is None:
suggestion_models: Sequence[GeneralSuggestionModel] = (
suggestion_query.fetch(offset=offset))
for suggestion_model in suggestion_models:
offset += 1
if suggestion_model.author_id != user_id:
sorted_results.append(suggestion_model)
else:
num_suggestions_per_fetch = 1000

while len(sorted_results) < limit:
suggestion_models: Sequence[GeneralSuggestionModel] = (
suggestion_query.fetch(
num_suggestions_per_fetch, offset=offset))
if not suggestion_models:
break
for suggestion_model in suggestion_models:
offset += 1
if suggestion_model.author_id != user_id:
sorted_results.append(suggestion_model)
if len(sorted_results) == limit:
break

return (
sorted_results,
offset
)

suggestion_query = cls.get_all().filter(datastore_services.all_of(
cls.status == STATUS_IN_REVIEW,
cls.suggestion_type == feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
Expand All @@ -539,9 +577,7 @@ def get_in_review_translation_suggestions_with_exp_ids_by_offset(
limit: Optional[int],
offset: int,
user_id: str,
# Currently, the argument sort_key is not used in this function, but it
# will be used when new sorting options are added in the frontend.
sort_key: Optional[str], # pylint: disable=unused-argument
sort_key: Optional[str],
language_codes: List[str],
exp_ids: List[str]
) -> Tuple[Sequence[GeneralSuggestionModel], int]:
Expand Down Expand Up @@ -571,6 +607,47 @@ def get_in_review_translation_suggestions_with_exp_ids_by_offset(
next_offset: int. The input offset + the number of results
returned by the current query.
"""
if sort_key == constants.SUGGESTIONS_SORT_KEY_DATE:
# The first sort property must be the same as the property to which
# an inequality filter is applied. Thus, the inequality filter on
# author_id can not be used here.
suggestion_query = cls.get_all().filter(datastore_services.all_of(
cls.status == STATUS_IN_REVIEW,
cls.suggestion_type == feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
cls.language_code.IN(language_codes),
cls.target_id.IN(exp_ids)
)).order(-cls.created_on)

sorted_results: List[GeneralSuggestionModel] = []

if limit is None:
suggestion_models: Sequence[GeneralSuggestionModel] = (
suggestion_query.fetch(offset=offset))
for suggestion_model in suggestion_models:
offset += 1
if suggestion_model.author_id != user_id:
sorted_results.append(suggestion_model)
else:
num_suggestions_per_fetch = 1000

while len(sorted_results) < limit:
suggestion_models: Sequence[GeneralSuggestionModel] = (
suggestion_query.fetch(
num_suggestions_per_fetch, offset=offset))
if not suggestion_models:
break
for suggestion_model in suggestion_models:
offset += 1
if suggestion_model.author_id != user_id:
sorted_results.append(suggestion_model)
if len(sorted_results) == limit:
break

return (
sorted_results,
offset
)

suggestion_query = cls.get_all().filter(datastore_services.all_of(
cls.status == STATUS_IN_REVIEW,
cls.suggestion_type == feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
Expand Down
180 changes: 180 additions & 0 deletions core/storage/suggestion/gae_models_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,98 @@ def test_get_translation_suggestions_in_review_with_exp_ids_by_offset(
suggestions[0].status,
suggestion_models.STATUS_IN_REVIEW)

def test_get_translation_suggestions_in_review_with_exp_ids_by_offset_sorted( # pylint: disable=line-too-long
self
) -> None:
suggestion_1_id = 'exploration.exp1.thread_6'
suggestion_2_id = 'exploration.exp1.thread_7'
suggestion_3_id = 'exploration.exp1.thread_8'
user_id = 'author1'
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change_cmd, self.score_category,
suggestion_1_id, self.translation_language_code)
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, 'author_4',
'reviewer_2', self.change_cmd, self.score_category,
suggestion_2_id, self.translation_language_code)
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, user_id,
'reviewer_2', self.change_cmd, self.score_category,
suggestion_3_id, self.translation_language_code)

sorted_results, offset_1 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_with_exp_ids_by_offset(
limit=1,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code],
exp_ids=['exp1']))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 1)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(offset_1, 2)

sorted_results, offset_2 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_with_exp_ids_by_offset(
limit=2,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code],
exp_ids=['exp1']))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_2, 3)

sorted_results, offset_3 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_with_exp_ids_by_offset(
limit=10,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code],
exp_ids=['exp1']))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_3, 3)

sorted_results, offset_4 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_with_exp_ids_by_offset(
limit=None,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code],
exp_ids=['exp1']))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_4, 3)

def test_get_in_review_translation_suggestions_by_offset(self) -> None:
suggestion_1_id = 'exploration.exp1.thread_6'
suggestion_2_id = 'exploration.exp1.thread_7'
Expand Down Expand Up @@ -586,6 +678,94 @@ def test_get_in_review_translation_suggestions_by_offset_no_limit(
self.assertEqual(results[1].id, suggestion_2_id)
self.assertEqual(offset, 2)

def test_get_in_review_translation_suggestions_by_offset_sorted(
self
) -> None:
suggestion_1_id = 'exploration.exp1.thread_6'
suggestion_2_id = 'exploration.exp1.thread_7'
suggestion_3_id = 'exploration.exp1.thread_8'
user_id = 'author1'
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, 'author_3',
'reviewer_2', self.change_cmd, self.score_category,
suggestion_1_id, self.translation_language_code)
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, 'author_4',
'reviewer_2', self.change_cmd, self.score_category,
suggestion_2_id, self.translation_language_code)
suggestion_models.GeneralSuggestionModel.create(
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
feconf.ENTITY_TYPE_EXPLORATION,
'exp1', self.target_version_at_submission,
suggestion_models.STATUS_IN_REVIEW, user_id,
'reviewer_2', self.change_cmd, self.score_category,
suggestion_3_id, self.translation_language_code)

sorted_results, offset_1 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_by_offset(
limit=1,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code]))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 1)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(offset_1, 2)

sorted_results, offset_2 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_by_offset(
limit=2,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code]))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_2, 3)

sorted_results, offset_3 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_by_offset(
limit=10,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code]))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_3, 3)

sorted_results, offset_4 = (
suggestion_models.GeneralSuggestionModel
.get_in_review_translation_suggestions_by_offset(
limit=None,
offset=0,
user_id=user_id,
sort_key=constants.SUGGESTIONS_SORT_KEY_DATE,
language_codes=[self.translation_language_code]))
# Ruling out the possibility of None for mypy type checking.
assert sorted_results is not None
self.assertEqual(len(sorted_results), 2)
self.assertEqual(sorted_results[0].id, suggestion_2_id)
self.assertEqual(sorted_results[1].id, suggestion_1_id)
self.assertEqual(offset_4, 3)

def test_get_in_review_question_suggestions_by_offset(self) -> None:
suggestion_1_id = 'skill1.thread1'
suggestion_2_id = 'skill1.thread2'
Expand Down
Loading