Skip to content
This repository

Grace periods not calculated correctly on the basis on the repo times #656

Closed
reidka opened this Issue February 12, 2012 · 14 comments

7 participants

Karen Reid baadshah02 Benjamin Vialle Christine Severin Gehwolf Sky Hu Michael Ing
Karen Reid
Owner

There are still ongoing issues with how grace periods are calculated.

I think the main problem is that the last commit time on the whole repo is used to establish grace period use, rather than the last commit to the relevant directory.

For example, suppose assignment 1 is due Friday at 10 pm, and students have 3 grace days. They also have a lab1 Monday morning, so they may end up with a repo that has both a1 and lab1. The a1 submissions view will make it look like the student has used 3 grace days even if they submit a1 before the Friday deadline. The student sees that they have used 3 grace days as well.

This issue involves the svn bindings that check the last commit time on the repo.

Christine
Collaborator

I can look into this. I must say I like the concept of grace days, never had them on assignments myself.

Severin Gehwolf
Owner

Like @reidka said, it will likely involve checking if the SVN bindings wrapper is doing thing correctly. I could see that it's checking the last commit to the entire repo instead of the repository folder of the given assignment.

Christine
Collaborator

In trying to reproduce this bug, I received an "ActionController::RoutingError in Results#edit" error. Steps:
1) Edit settings for a1 to "Automatically deduct grace period credits"
2) Give 10 grace credits to user c5anthei
3) Submit file to a1 as c5anthei
4) Login as admin, attempt to view c5anthei's submission, get RoutingError:
ActionController::RoutingError in Results#edit

Showing /home/usrname/workspace/Markus/app/views/submission_rules/grace_period/_grader_tab.html.erb where line #18 raised:

No route matches {:action=>"delete_grace_period_deduction", :controller=>"results", :deduction_id=>1, :id=>16}

Is this a new issue?

Severin Gehwolf
Owner

Looks like a new one to me. Please file it as a new one.

Christine
Collaborator

Having trouble reproducing this bug... @reidka, to confirm, in the given scenario is the Monday of the lab in the same week as the Friday due date of the assignment (L1 past due date), or is it the following Monday (L1 not due yet)?

Karen Reid
Owner

I've seen it happen in the following scenario:
A1 due Friday 10 pm Feb 24. Three grace days are allowed.

A student submits before 10 pm on Friday (no grace days used)

If the student is also on top of things with the labs and submits the lab on Monday afternoon, and we haven't collected A1 yet, it will appear as though the student has used 3 grace days for A1.

baadshah02
Collaborator

Thanks, @reidka. I was able to recreate the issue by giving it 0.25 hours of grace credits and then collecting the assignment. Severin is right. It's looking at the overall commit time when we collect the assignment. Can you assign me to this issue. I'd like to work on it now that I have reproduced it! :)

Severin Gehwolf
Owner

@baadshah02 Not sure if you've seen my comment on this review: http://review.markusproject.org/r/1161/ (I was confusing @lakeskysea and you). Perhaps it's helpful.

Sky Hu
Collaborator

@jerboaa sorry for the confusion :S

  1. lakeskysea is Sky (me), @baadshah02 is Jay (if I am not wrong)

  2. http://review.markusproject.org/r/1161/ is a different issue (#320) from this one (#656)

#320: submission view shows graceday credits used by the entire group; it should be changed to show graceday used by each single member. (shipped)

#656: deals with repo commit time

Sky Hu
Collaborator

@baadshah02 you can check the comments @jerboaa left at the bottom of the page http://review.markusproject.org/r/1161/ I think it's for you :)

baadshah02
Collaborator

@lakeskysea, Thanks a lot! @jerboaa, I read yr comments. I was able to reproduce it, so I have seen the issue. Let me try that scenario when I don't submit assignment for a1 but submit it for a2 and see if it uses up a grace day for a1. After i reproduced the issue, my initial step was to sort of browse around the code to understand how it's setting the commit time when an assignment is collected. Need to spend some more time understanding it before i make changes to the code. I'll get back once I've some progress. If you have any other suggestions, I welcome them. Thanks!! :)

Severin Gehwolf
Owner

@baadshah02 OK. FYI: The idea was to write the test first (it should be very similar to @lakeskysea's test). Just submit to a different assignment, collect and then check grace credits. This should give you a failing test. Once you have that you have all your testing already done (I will ask for it anyway ;-)). The test would then succeed once you've fixed the code. Why bother doing this? Because we don't want this issue to crop up again in later releases. An automated test for this would assert that.

Michael Ing
Collaborator

Is anyone currently working on this issue? I have a fix for this issue.

baadshah02
Collaborator

yes i'm working on this but i haven't had much time yet after my pey term started and my markus database broke down. So I still need to fix that. If you have a fix for it, you can definitely go ahead and submit it on review board. :)

Benjamin Vialle benjaminvialle closed this in 5cfe934 June 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.