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.

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 #4579: Prevent hints from being released after correct answer is submitted. #4580

Merged
merged 1 commit into from
Jan 21, 2018

Conversation

tjiang11
Copy link
Contributor

#4579

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

@tjiang11 tjiang11 requested a review from seanlip January 20, 2018 22:31
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @tjiang11!

@DubeySandeep, could you please do a second review?

@seanlip
Copy link
Member

seanlip commented Jan 20, 2018

Also, @tjiang11 -- any chance we could add Karma tests for this? Not sure how feasible it is with the $on stuff, but might be worth trying.

@tjiang11
Copy link
Contributor Author

It's probably doable, won't be able to take a look again until later tonight however

Copy link
Member

@DubeySandeep DubeySandeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjiang11, LGTM!
Thanks! :)

@codecov-io
Copy link

Codecov Report

Merging #4580 into develop will increase coverage by <.01%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4580      +/-   ##
===========================================
+ Coverage    44.76%   44.77%   +<.01%     
===========================================
  Files          382      382              
  Lines        23197    23202       +5     
  Branches      3724     3725       +1     
===========================================
+ Hits         10385    10389       +4     
- Misses       12812    12813       +1
Impacted Files Coverage Δ
...es/exploration_player/ConversationSkinDirective.js 1.96% <0%> (ø) ⬆️
...ploration_player/HintsAndSolutionManagerService.js 76.69% <87.5%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bd47d7...b7524ea. Read the comment docs.

@seanlip
Copy link
Member

seanlip commented Jan 21, 2018

OK, thanks @tjiang11! Let me bring this in for now and do a hotfix -- it'd be great if we could add the karma test in later.

@seanlip seanlip merged commit 7b2fb13 into oppia:develop Jan 21, 2018
giritheja added a commit to giritheja/oppia that referenced this pull request Jan 23, 2018
* upstream/develop: (77 commits)
  Add test to HintAndSolutionManagerService to check that hints aren't released after a correct answer is submitted. (oppia#4585)
  Chance cancel property in SpeechSynthesisChunkerService into its own variable. (oppia#4591)
  Enable indent for func-expr (oppia#4588)
  Update text of refresher exp modal. (oppia#4584)
  Update content-type for JSON responses. (oppia#4583)
  Prevent hints from being released after correct answer is submitted. (oppia#4580)
  Change fraction placeholder when no integer part is allowed. (oppia#4581)
  Fix oppia#4567: Changed condition to check range enclosed criteria, edited existing test (oppia#4575)
  Fix part of oppia#3905: Added semi-rule (oppia#4576)
  Adds proper message for validation error in skill ID. (oppia#4577)
  Fix part of oppia#3905: Add checks for correct docstring style. (oppia#4572)
  Fix part of oppia#4374: Add docstrings to core.domain.collection_domain. (oppia#4569)
  Add check to prevent 'SpeechSynthesisUtterance is undefined' error. (oppia#4571)
  Fix of oppia#2533: Modified the stripFormatting filter to preserve newlines in RTE cut-and-paste (oppia#4568)
  Fix part of oppia#3905: Add check for console.log statement (oppia#4564)
  Change Java prerequisite to v8 (oppia#4570)
  Fix oppia#1542: RTE Components correctly displayed in answer group header (oppia#4565)
  Fix part of oppia#4374: Add docstrings to core.domain.param_domain (oppia#4551)
  Fix oppia#4555: Address review comments for oppia#4536. (oppia#4556)
  Update changelog, AUTHORS, CONTRIBUTORS and credits. (oppia#4560)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants