Skip to content

Commit

Permalink
Add paging for questions in topic editor and story editor modificatio…
Browse files Browse the repository at this point in the history
…ns (oppia#5892)

* bug fixes

* added show/hide of story notes editor

* Added cursor management logic

* added paging in questions tab in topic editor

* fix lint

* fixed question indexing

* fix lint

* changed sorting order for questions

* Fixed backend test error

* added comment
  • Loading branch information
aks681 authored and seanlip committed Nov 28, 2018
1 parent cf4939c commit 13074c3
Show file tree
Hide file tree
Showing 14 changed files with 169 additions and 42 deletions.
2 changes: 2 additions & 0 deletions assets/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,8 @@ var constants = {

"ENABLE_NEW_STRUCTURES": false,

"NUM_QUESTIONS_PER_PAGE": 10,

"NEW_STATE_TEMPLATE": {
"classifier_model_id": null,
"content": {
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/topic_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def get(self, topic_id):

question_summaries, next_start_cursor = (
question_services.get_question_summaries_linked_to_skills(
feconf.NUM_QUESTIONS_PER_PAGE, skill_ids, start_cursor)
constants.NUM_QUESTIONS_PER_PAGE, skill_ids, start_cursor)
)
question_summary_dicts = [
summary.to_dict() for summary in question_summaries]
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/topic_editor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_get(self):

with self.swap(constants, 'ENABLE_NEW_STRUCTURES', True):
self.login(self.ADMIN_EMAIL)
with self.swap(feconf, 'NUM_QUESTIONS_PER_PAGE', 1):
with self.swap(constants, 'NUM_QUESTIONS_PER_PAGE', 1):
json_response = self.get_json(
'%s/%s?cursor=' % (
feconf.TOPIC_EDITOR_QUESTION_URL, self.topic_id
Expand Down
7 changes: 5 additions & 2 deletions core/storage/question/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,17 @@ def get_question_ids_linked_to_skill_ids(
cursor = datastore_query.Cursor(urlsafe=start_cursor)
question_skill_link_models, next_cursor, more = cls.query(
cls.skill_id.IN(skill_ids)
).order(cls.key).fetch_page(
# Order by cls.key is needed alongside cls.last_updated so as to
# resolve conflicts, if any.
# Reference SO link: https://stackoverflow.com/q/12449197
).order(-cls.last_updated, cls.key).fetch_page(
question_count,
start_cursor=cursor
)
else:
question_skill_link_models, next_cursor, more = cls.query(
cls.skill_id.IN(skill_ids)
).order(cls.key).fetch_page(
).order(-cls.last_updated, cls.key).fetch_page(
question_count
)
question_ids = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ oppia.constant(

oppia.constant(
'TOPIC_EDITOR_QUESTION_URL_TEMPLATE',
'/topic_editor_question_handler/<topic_id>');
'/topic_editor_question_handler/<topic_id>?cursor=<cursor>');

oppia.factory('EditableTopicBackendApiService', [
'$http', '$q', 'EDITABLE_TOPIC_DATA_URL_TEMPLATE',
Expand Down Expand Up @@ -77,17 +77,22 @@ oppia.factory('EditableTopicBackendApiService', [
};

var _fetchQuestions = function(
topicId, successCallback, errorCallback) {
topicId, cursor, successCallback, errorCallback) {
var questionsDataUrl = UrlInterpolationService.interpolateUrl(
TOPIC_EDITOR_QUESTION_URL_TEMPLATE, {
topic_id: topicId
topic_id: topicId,
cursor: cursor ? cursor : ''
});

$http.get(questionsDataUrl).then(function(response) {
var questionSummaries = angular.copy(
response.data.question_summary_dicts);
var nextCursor = response.data.next_start_cursor;
if (successCallback) {
successCallback(questionSummaries);
successCallback({
questionSummaries: questionSummaries,
nextCursor: nextCursor
});
}
}, function(errorResponse) {
if (errorCallback) {
Expand Down Expand Up @@ -176,9 +181,9 @@ oppia.factory('EditableTopicBackendApiService', [
});
},

fetchQuestions: function(topicId) {
fetchQuestions: function(topicId, cursor) {
return $q(function(resolve, reject) {
_fetchQuestions(topicId, resolve, reject);
_fetchQuestions(topicId, cursor, resolve, reject);
});
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ oppia.directive('skillConceptCardEditor', [
var initBindableFieldsDict = function() {
$scope.bindableFieldsDict = {
displayedConceptCardExplanation:
$scope.skill.getConceptCard().getExplanation(),
$scope.skill.getConceptCard().getExplanation().getHtml(),
displayedWorkedExamples:
$scope.skill.getConceptCard().getWorkedExamples()
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ oppia.directive('storyEditor', [
$scope.disconnectedNodeIds =
$scope.storyContents.getDisconnectedNodeIds();
}
$scope.notesEditorIsShown = false;
$scope.storyTitleEditorIsShown = false;
$scope.editableTitle = $scope.story.getTitle();
$scope.editableNotes = $scope.story.getNotes();
$scope.editableDescription = $scope.story.getDescription();
Expand All @@ -62,6 +64,14 @@ oppia.directive('storyEditor', [
$scope.idOfNodeToEdit = nodeId;
};

$scope.openNotesEditor = function() {
$scope.notesEditorIsShown = true;
};

$scope.closeNotesEditor = function() {
$scope.notesEditorIsShown = false;
};

$scope.isInitialNode = function(nodeId) {
return (
$scope.story.getStoryContents().getInitialNodeId() === nodeId);
Expand Down Expand Up @@ -164,6 +174,7 @@ oppia.directive('storyEditor', [
return;
}
StoryUpdateService.setStoryNotes($scope.story, newNotes);
_initEditor();
};

$scope.updateStoryDescriptionStatus = function(description) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,43 @@
</div>

<div class="story-notes">
<label class="form-heading">Notes</label>
<div class="oppia-editor-card-body">
<schema-based-editor schema="NOTES_SCHEMA"
local-value="editableNotes">
</schema-based-editor>
<button type="button"
class="btn btn-success save-button"
ng-disabled="!editableNotes || (editableNotes === story.getNotes())"
ng-click="updateNotes(editableNotes)">
Save
</button>
<label class="form-heading">Notes</label>
<div ng-if="!notesEditorIsShown" style="padding-top: 0.5em; padding-bottom: 1em;">
<div ng-class="oppia-editable-section"
ng-click="openNotesEditor()">
<i class="material-icons oppia-editor-edit-icon pull-right"
title="Edit Story Notes">&#xE254;
</i>
<div class="oppia-state-content-display oppia-transition-200"
ng-class="oppia-prevent-selection"
title="Story notes">
<span class="oppia-placeholder" ng-show="editableNotes === ''">
Add notes about the story to help other contributors.
</span>
<angular-html-bind html-data="editableNotes">
</angular-html-bind>
</div>
<!-- This is a dummy div created to mask the contents when the user hovers over the content. -->
<div class="oppia-editable-section-mask">
</div>
</div>
</div>

<div ng-if="notesEditorIsShown">
<schema-based-editor schema="NOTES_SCHEMA" local-value="editableNotes">
</schema-based-editor>
<div class="editor-buttons">
<button type="button"
class="btn btn-success oppia-save-state-item-button pull-right"
ng-disabled="!editableNotes"
ng-click="updateNotes(editableNotes)">
Save
</button>
<button type="button" class="btn btn-default pull-right" ng-click="closeNotesEditor()">Cancel</button>
<div style="clear: both;"></div>
</div>
</div>
</div>
</div>
<div class="story-nodes-title">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ oppia.factory('TopicEditorStateService', [
var _topicIsBeingSaved = false;
var _canonicalStorySummaries = [];
var _questionSummaries = [];
var _nextCursorForQuestions = '';

var _getSubtopicPageId = function(topicId, subtopicId) {
return topicId + '-' + subtopicId.toString();
Expand Down Expand Up @@ -110,9 +111,12 @@ oppia.factory('TopicEditorStateService', [
$rootScope.$broadcast(EVENT_STORY_SUMMARIES_INITIALIZED);
};
var _setQuestionSummaries = function(questionSummaries) {
_questionSummaries = angular.copy(questionSummaries);
_questionSummaries.push(angular.copy(questionSummaries));
$rootScope.$broadcast(EVENT_QUESTION_SUMMARIES_INITIALIZED);
};
var _setNextQuestionsCursor = function(nextCursor) {
_nextCursorForQuestions = nextCursor;
};

return {
/**
Expand All @@ -133,10 +137,13 @@ oppia.factory('TopicEditorStateService', [
function(canonicalStorySummaries) {
_setCanonicalStorySummaries(canonicalStorySummaries);
});
EditableTopicBackendApiService.fetchQuestions(topicId).then(
function(questionSummaries) {
_setQuestionSummaries(questionSummaries);
});
EditableTopicBackendApiService.fetchQuestions(
topicId, _nextCursorForQuestions).then(
function(returnObject) {
_setQuestionSummaries(returnObject.questionSummaries);
_setNextQuestionsCursor(returnObject.nextCursor);
}
);
},
function(error) {
AlertsService.addWarning(
Expand Down Expand Up @@ -186,6 +193,12 @@ oppia.factory('TopicEditorStateService', [
return _topicIsLoading;
},

isLastQuestionBatch: function(index) {
return (
_nextCursorForQuestions === null &&
index === _questionSummaries.length - 1);
},

/**
* Returns whether a topic has yet been loaded using either
* loadTopic() or setTopic().
Expand All @@ -210,15 +223,25 @@ oppia.factory('TopicEditorStateService', [
return _canonicalStorySummaries;
},

getQuestionSummaries: function() {
return _questionSummaries;
fetchQuestionSummaries: function(topicId, resetHistory) {
if (resetHistory) {
_questionSummaries = [];
_nextCursorForQuestions = '';
}
EditableTopicBackendApiService.fetchQuestions(
topicId, _nextCursorForQuestions).then(
function(returnObject) {
_setQuestionSummaries(returnObject.questionSummaries);
_setNextQuestionsCursor(returnObject.nextCursor);
}
);
},

fetchQuestionSummaries: function(topicId) {
EditableTopicBackendApiService.fetchQuestions(topicId).then(
function(questionSummaries) {
_setQuestionSummaries(questionSummaries);
});
getQuestionSummaries: function(index) {
if (index >= _questionSummaries.length) {
return null;
}
return _questionSummaries[index];
},

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,17 @@ oppia.directive('questionsTab', [
'EditableQuestionBackendApiService', 'EditableSkillBackendApiService',
'MisconceptionObjectFactory', 'QuestionObjectFactory',
'QuestionSuggestionObjectFactory', 'SuggestionThreadObjectFactory',
'EVENT_QUESTION_SUMMARIES_INITIALIZED', 'StateEditorService', function(
'EVENT_QUESTION_SUMMARIES_INITIALIZED', 'StateEditorService',
'NUM_QUESTIONS_PER_PAGE', function(
$scope, $http, $q, $uibModal, $window, AlertsService,
TopicEditorStateService, QuestionCreationService, UrlService,
EditableQuestionBackendApiService, EditableSkillBackendApiService,
MisconceptionObjectFactory, QuestionObjectFactory,
QuestionSuggestionObjectFactory, SuggestionThreadObjectFactory,
EVENT_QUESTION_SUMMARIES_INITIALIZED, StateEditorService) {
EVENT_QUESTION_SUMMARIES_INITIALIZED, StateEditorService,
NUM_QUESTIONS_PER_PAGE) {
$scope.currentPage = 0;

var _initTab = function() {
$scope.questionEditorIsShown = false;
$scope.question = null;
Expand All @@ -44,13 +48,37 @@ oppia.directive('questionsTab', [
$scope.topicRights = TopicEditorStateService.getTopicRights();
$scope.canEditQuestion = $scope.topicRights.canEditTopic();
$scope.questionSummaries =
TopicEditorStateService.getQuestionSummaries();
TopicEditorStateService.getQuestionSummaries($scope.currentPage);
$scope.isLastPage = TopicEditorStateService.isLastQuestionBatch;
$scope.misconceptions = [];
$scope.questionSuggestionThreads = [];
$scope.activeQuestion = null;
$scope.suggestionReviewMessage = null;
};

$scope.getQuestionIndex = function(index) {
return $scope.currentPage * NUM_QUESTIONS_PER_PAGE + index + 1;
};

$scope.goToNextPage = function() {
$scope.currentPage++;
var questionSummaries =
TopicEditorStateService.getQuestionSummaries($scope.currentPage);
if (questionSummaries === null) {
TopicEditorStateService.fetchQuestionSummaries(
$scope.topic.getId(), false
);
} else {
$scope.questionSummaries = questionSummaries;
}
};

$scope.goToPreviousPage = function() {
$scope.currentPage--;
$scope.questionSummaries =
TopicEditorStateService.getQuestionSummaries($scope.currentPage);
};

$scope.saveAndPublishQuestion = function() {
var validationErrors = $scope.question.validate(
$scope.misconceptions);
Expand All @@ -62,8 +90,9 @@ oppia.directive('questionsTab', [
$scope.skillId, $scope.question.toBackendDict(true)
).then(function() {
TopicEditorStateService.fetchQuestionSummaries(
$scope.topic.getId()
$scope.topic.getId(), true
);
$scope.currentPage = 0;
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ <h3 class="form-heading"> Questions included in this topic </h3>
</tr>
<tr ng-repeat="question in questionSummaries"
class="list-item">
<td> <[$index + 1]> </td>
<td> <[getQuestionIndex($index)]> </td>
<td>
<a class="list-summary">
<angular-html-bind html-data="question.question_content"></angular-html-bind>
Expand Down Expand Up @@ -63,6 +63,13 @@ <h3 class="form-heading" ng-if="questionSuggestionThreads.length > 0"> Questions
</td>
</tr>
</table>
<div class="page-navigation-arrows" ng-if="questionSummaries.length > 0">
<i class="material-icons md-18"
ng-if="currentPage !== 0"
ng-click="goToPreviousPage()">&#xE5C4;
</i> Page <[currentPage + 1]>
<i class="material-icons md-18" ng-if="!isLastPage(currentPage)" ng-click="goToNextPage()">&#xE5C8;</i>
</div>
</div>
</md-card>

Expand All @@ -86,6 +93,10 @@ <h3 class="form-heading" ng-if="questionSuggestionThreads.length > 0"> Questions
width: 80%;
}

questions-tab .questions {
padding-bottom: 10px;
}

questions-tab .oppia-editor-publish-button {
margin-top: 2%;
}
Expand All @@ -96,6 +107,10 @@ <h3 class="form-heading" ng-if="questionSuggestionThreads.length > 0"> Questions
width: 100%;
}

questions-tab .page-navigation-arrows {
float: right;
}

questions-tab .list-summary,
questions-tab .list-summary:active,
questions-tab .list-summary:visited {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
display: block;
font-size: 15px;
max-height: 250px;
overflow-y: scroll;
overflow-y: auto;
width: 100%;
}

Expand Down
Loading

0 comments on commit 13074c3

Please sign in to comment.