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 for issue 1013 and 1020 - also includes fixes to related bugs that were found (not logged) #1064

Merged
merged 3 commits into from Apr 8, 2013

Conversation

Projects
None yet
2 participants
@akwok18
Contributor

akwok18 commented Apr 3, 2013

Fixes issue #1013 - included a new partial view for a non-editable result summary view for the extra percentage table

Came across the following issues while working on this (also fixed):

  • invalid method call if extra percentage was from Penalty Decay setting
  • missing UI text if extra percentage was from Penalty Decay setting
  • fixed "remove" update of text (translation to points was not being updated after removal)

Added fix to related issue #1020 - rollover of "Show Old Mark Summary" correctly displays old mark summary for penalties/bonuses

Show outdated Hide outdated app/views/results/student/_student_summary.html.erb
<div id="extra_marks_pane" class="extra_marks_pane">
<div class="bonus_deduction"><strong><%= I18n.t("marker.marks.bonus_deductions") %></strong></div>
<<<<<<< Updated upstream

This comment has been minimized.

@benjaminvialle

benjaminvialle Apr 5, 2013

Member

Conflict here ;)

@benjaminvialle

benjaminvialle Apr 5, 2013

Member

Conflict here ;)

Show outdated Hide outdated app/views/results/student/_student_summary.html.erb
<div class="clear"></div>
=======
>>>>>>> Stashed changes

This comment has been minimized.

@benjaminvialle

benjaminvialle Apr 5, 2013

Member

Same thing here.

@benjaminvialle

benjaminvialle Apr 5, 2013

Member

Same thing here.

@benjaminvialle

This comment has been minimized.

Show comment
Hide comment
@benjaminvialle

benjaminvialle Apr 6, 2013

Member

Ok. The code is good.

Could you add some tests and I will merge it!

Member

benjaminvialle commented Apr 6, 2013

Ok. The code is good.

Could you add some tests and I will merge it!

@akwok18

This comment has been minimized.

Show comment
Hide comment
@akwok18

akwok18 Apr 8, 2013

Contributor

Hi Benjamin,
What kind of tests would you like me to add? I've only modified views to incorporate either calling the methods on an original result or a remark result. These methods already have unit tests written for them.

Contributor

akwok18 commented Apr 8, 2013

Hi Benjamin,
What kind of tests would you like me to add? I've only modified views to incorporate either calling the methods on an original result or a remark result. These methods already have unit tests written for them.

@benjaminvialle

This comment has been minimized.

Show comment
Hide comment
@benjaminvialle

benjaminvialle Apr 8, 2013

Member

Yes, you're right :)

Member

benjaminvialle commented Apr 8, 2013

Yes, you're right :)

benjaminvialle added a commit that referenced this pull request Apr 8, 2013

Merge pull request #1064 from akwok18/issue-1013
Includes fixes to related bugs that were found (not logged)
Closes #1013 ; Closes #1020

@benjaminvialle benjaminvialle merged commit b06d85f into MarkUsProject:master Apr 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment