FIX for issue 941 updated database schema for submissions and results. #1026

Merged
merged 3 commits into from Mar 6, 2013

Conversation

Projects
None yet
3 participants
@akwok18
Contributor

akwok18 commented Mar 4, 2013

Submissions now have a one to many relationship with results (instead of one to one result and one to one remark result). Implemented methods to retrieve the correct result (remark or original) that are not relying on database ordering or timestamp. Updated blueprints and tests for updated schema. Functionality should remain the same as before the schema change.

Fix for issue #941

FIX for issue 941 updated database schema for submissions and results…
…. Helper methods implemented to retrieve appropriate result.
app/helpers/submissions_helper.rb
+ raise I18n.t("marking_state.not_complete", :group_name => grouping.group.group_name) if submission.get_original_result.marking_state != Result::MARKING_STATES[:complete]
+ @result = submission.get_original_result
+ @result.released_to_students = release
+ @result.save

This comment has been minimized.

@m-wu

m-wu Mar 6, 2013

Contributor

result is being saved twice on the next line.

@m-wu

m-wu Mar 6, 2013

Contributor

result is being saved twice on the next line.

app/models/submission.rb
+ # returns the original result
+ def get_original_result
+ if self.remark_result_id.nil?
+ result = Result.find(:all, :conditions => ["submission_id = ?", self.id]).first

This comment has been minimized.

@m-wu

m-wu Mar 6, 2013

Contributor

I'm not sure, but maybe using Result.find(:first, _args) or its shortcut Result.first(_args) would be more efficient?

@m-wu

m-wu Mar 6, 2013

Contributor

I'm not sure, but maybe using Result.find(:first, _args) or its shortcut Result.first(_args) would be more efficient?

assert_not_nil next_result
assert_not_nil previous_result
+ debugger

This comment has been minimized.

@m-wu

m-wu Mar 6, 2013

Contributor

We probably don't need this for everyone. :)

@m-wu

m-wu Mar 6, 2013

Contributor

We probably don't need this for everyone. :)

@m-wu

This comment has been minimized.

Show comment
Hide comment
@m-wu

m-wu Mar 6, 2013

Contributor

These changes do not cause any additional failures in the test suite, and besides the comments I made above, the pull request looks good to me. (But since I'm still relatively new to RoR, a quick scan by a pro wouldn't hurt.)

Contributor

m-wu commented Mar 6, 2013

These changes do not cause any additional failures in the test suite, and besides the comments I made above, the pull request looks good to me. (But since I'm still relatively new to RoR, a quick scan by a pro wouldn't hurt.)

reidka added a commit that referenced this pull request Mar 6, 2013

Merge pull request #1026 from akwok18/issue-941
FIX for issue 941 updated database schema for submissions and results.
I'm going to merge this one so that work can continue on this area.  There is lots of active development on this part of the code right now so hopeful any issues with it will appear soon.

@reidka reidka merged commit e980bbf into MarkUsProject:master Mar 6, 2013

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