-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Implement editing of questions #5614
Merged
Merged
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
d9f2824
Save.
tjiang11 f0cb957
Save.
tjiang11 9c8e702
Save. UrlInterpolationError topic null.
tjiang11 c93f186
Save.
tjiang11 f695dd6
Modify backend to return associated skills when fetching a question.
tjiang11 066faec
save.
tjiang11 07a11fb
Saving content works.
tjiang11 2386f69
Copy interaction.
tjiang11 4360648
Apply QuestionUpdateService for all changes.
tjiang11 a68adef
Circumvent race condition on initialization.
tjiang11 dd9d25f
Lint.
tjiang11 8a2c2bd
Clone UndoRedoService for questions.
tjiang11 4782099
Fetch question summaries after edit question.
tjiang11 a20b0ab
Create reusable function in QuestionEditorDirective.
tjiang11 4668f51
Lint.
tjiang11 fd91189
Add test.
tjiang11 2f4f1cc
Fix test.
tjiang11 3050886
Lint.
tjiang11 6d52d8c
Disable flag.
tjiang11 43807b1
Review changes.
tjiang11 7a4ff6f
Review changes.
tjiang11 41798c6
Review comments.
tjiang11 c496793
Lint.
tjiang11 fbdaa83
Review changes.
tjiang11 732be38
Deduplicate UndoRedoService.
tjiang11 7756f6d
Move updateFunction into QuestionUpdateService.
tjiang11 17ca89f
Fix test.
tjiang11 5785784
Lint.
tjiang11 13254ea
Update documentation.
tjiang11 fc4981d
Add tests for get_models_by_question_id
tjiang11 9ef2940
Update setQuestionStateData.
tjiang11 5861a5a
Fix test.
tjiang11 4ccea9c
Fix test.
tjiang11 0c589eb
Merge develop.
tjiang11 c0ad966
Lint.
tjiang11 9896d99
Add test.
tjiang11 4d3c329
Merge develop.
tjiang11 d37ab38
Lint.
tjiang11 f6f7e09
Edit questions in modal.
tjiang11 5722ca1
Add blank commit message.
tjiang11 3b5983f
Remove duplicate function; Fix issue with not being able to edit a qu…
tjiang11 c4c9b93
Merge develop.
tjiang11 98227d0
Lint.
tjiang11 5abc2c8
Lint.
tjiang11 387a9cb
Fix.
tjiang11 7a10a7b
Flip flag.
tjiang11 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Apply QuestionUpdateService for all changes.
- Loading branch information
commit 4360648a9451c7ec257b19a773270714041e22af
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,8 +201,6 @@ def get_multi_skills(skill_ids): | |
Returns: | ||
list(Skill). The list of skills matching the provided IDs. | ||
""" | ||
print "\n\n\n\n\n\n\n\n" | ||
print get_skill_by_id(skill_ids[0]) | ||
local_skill_models = skill_models.SkillModel.get_multi(skill_ids) | ||
skills = [ | ||
get_skill_from_model(skill_model) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if any of the skill_id doesn't exist? In that case, won't a None be returned? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to reflect the implementation just excluding skills that are None.
Either throw an exception if a skill is requested that does not exist, or return None in the appropriate position; probably the former is better if you don't expect this to occur in practice. But just silently removing stuff altogether seems like it's not the right thing to do -- you'll end up with the length of the input list not matching the output list which is something the caller won't expect given the function contract.
There should also be tests for this error case, it's quite important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by raising Exception