[Building Sakai] weird bug in assignments: it's showing the

[Jeon,Sanghyun Seo]

Thank you for all your efforts.
I am out of town and will be back next Wednesday. I will apply your solution when I am back and I am expecting it will resolve our problem, too.
Thank you.

Sanghyun


________________________________
From: Hedrick Charles [hedrick at rutgers.edu]
Sent: Wednesday, February 23, 2011 10:22 PM
To: Hedrick Charles
Cc: Jeon,Sanghyun Seo; Zhen Qian; sakai-dev at collab.sakaiproject.org
Subject: Re: [Building Sakai] weird bug in assignments: it's showing the

I believe what happened to us is that the user was seeing slow response (because they were on wireless) and clicked next twice. My suspicion is that this somehow resulted in two submissions of the same form data, when in effect simulated the two window scenario below. Given that we've now seen this in two sites from at least two different people in the last week, I feel we have to go beyond just a warning. A warning probably isn't going to be good enough to prevent the double click problem, since users quite often suspect their click didn't register if response is slower than expected. Trying to fix that with warnings is going to be nearly impossible.

While I think it still makes sense to warn users about not using two windows, we can certainly fix the code so it won't apply data to the wrong submission ID.

On Feb 23, 2011, at 10:03:27 PM, Hedrick Charles wrote:

Given this sample, the bug is pretty obvious.

In step 5, window X does "next". That calls grade_submission_option.

That takes the submission ID from the state. It has now been advanced to student B. However it takes it data from the POST, which will be data from student A.

readGradeForm copies data from the POST parameters into the state. But it doesn't copy the submission ID into the state, although it uses it for internal computations. I can see two possible fixes:

* in readGradeForm, copy the submission ID from the POST into the state
* in readGradeForm, if the submission ID from the POST is different from the submission ID in the state, issue a fatal error and abort, although possibly returning false is good enough.

The first solution is the nicest, but I'm concerned that there may be other inconsitencies.

The following patch is safe, in the sense that it won't save bad data, and produces a warning message. However it fails to save the new data in window X.

Index: assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
===================================================================
--- assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java (revision 2850)
+++ assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java (working copy)
@@ -7859,6 +7859,11 @@
  ParameterParser params = data.getParameters();
  String sId = params.getString("submissionId");

+ if (!sId.equals((String) state.getAttribute(GRADE_SUBMISSION_SUBMISSION_ID))) {
+    addAlert(state, "Inconsistent state. Normally this occurs if you are using two windows or have hit a button more than once. To avoid the danger of producing corrupted data, we are not doing what you asked for. Please go back to the assignment list without doing anything that would save your current work.");
+    return false;
+ }
+
  // security check for allowing grading submission or not
  if (AssignmentService.allowGradeSubmission(sId))
  {


Here's the fix you really want:

Index: assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
===================================================================
--- assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java (revision 2850)
+++ assignment-tool/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java (working copy)
@@ -7859,6 +7859,8 @@
  ParameterParser params = data.getParameters();
  String sId = params.getString("submissionId");

+ state.setAttribute(GRADE_SUBMISSION_SUBMISSION_ID, sId);
+
  // security check for allowing grading submission or not
  if (AssignmentService.allowGradeSubmission(sId))
  {

Both fix the problem in my testing. I'm hoping the assignment experts will look into this and see what they think is the safest fix. I'd like to see one or the other implemented post haste. I'm probably going to deploy one or the other this weekend at Rutgers.


On Feb 23, 2011, at 3:25:00 PM, Jeon,Sanghyun Seo wrote:

I can duplicate the problem successfully as follows:
1.       Open two windows separately: Windows X and Y
2.       Click “ view Submissions” in Windows X
3.       Click “Grade” under Student A in Windows X and type something in Instructor Summary Comments Box
4.       Just click “Grade” under Student B in Windows Y
5.       Back to Windows X and hit “Next”. You will see the Student A’s content under Student B, which is wrong

Even though you will open new windows and try it again, the Student A’s content with instructor’s comment is displayed under Student B when you login in as instructor, but the correct content will be presented when you login in as each student.


Sanghyun

From: Charles Hedrick [mailto:hedrick at rutgers.edu]
Sent: Wednesday, February 23, 2011 2:41 PM
To: Zhen Qian
Cc: Jeon,Sanghyun Seo; sakai-dev at collab.sakaiproject.org<mailto:sakai-dev at collab.sakaiproject.org>
Subject: Re: [Building Sakai] weird bug in assignments: it's showing the

I wouldn't expect you to be able to duplicate the problem. It occurs randomly in a few small percentage of cases. It's either truly random or depends upon specific timing of events.

We have seen it i both IE 7 and Firefox, so it's not the browser.

Here are the database transactions for the second submission, i.e. the one that got the bad data:

insert lock
update submission, correct
delete lock
sakai event asn.grade.submission
lock
update submission, with bad feedback entry
delete lock
event asn.grade.submission
event asn.read.submission

Interestingly the previous one shows an update and asn.grade.submission without a lock/unlock, possibly because no change was made?


On Feb 23, 2011, at 2:28 PM, Zhen Qian wrote:



Chuck:

We are currently running 2.7.1 with extra patches.

However, I cannot repeat the problem by following the step below. Could you provide more details here?

Thanks,

- Zhen



On Wed, 23 Feb 2011 14:15:14 -0500, Charles Hedrick <hedrick at rutgers.edu<mailto:hedrick at rutgers.edu>> wrote:

OK, so it's now happening at at least 3 places. Any commonality in sakai versions? We hadn't seen it under 2.7.0, but are seeing it in 2.7.1.
Yes, they are inline text, not attachments.
What's happening is the following:
instructor hits "next"
new screen comes up with text from old screen in FCK
instructor does next or previous
an automatic save occurs. The contents of FCK are saved as feedback, i.e. Sakai thinks it's a version of the student's assignment with instructor comments
The database shows the correct submission, but the wrong one as feedback.
If you go back to the assignment in the UI, you'll see the wrong data, because when feedback exists, the UI will show it rather than the original submission.
We think other data such as grade may also be duplicated, but we can't be sure yet.
I don't see how having another window open would have any effect. The instructor is going from one student to another in the same window. I understand that doing that isn't safe, but it wold require a very strange and unlikely pattern of actions in the two windows to cause this effect.
It's happened enough that I consider it a crisis.

On Feb 23, 2011, at 12:38 PM, Jeon,Sanghyun Seo wrote:

Hello,

We, University of Florid, are having the exact same problem, which happened yesterday.
That’s for inline assignments, not attachments as well as unlimited submissions.
The instructor claimed that he knew there was a problem as soon as he opened it, because the student names did not match up.
He even did not try to give any inline comments or scores due to this error.
But he admitted that there could have been another browser open.

My question is that which database table stores student’s view content and which one stores instructor’s view content.
When I logged in as Student A, Student A’s submission contains “content A”, and Student B’s submission contains “content B”, which are correct.
However,
when I logged in as instructor, then Student A had “content A”, which is right, but Student B also has “content A”, which is wrong.
Since student’s view contents are correct for both case, but instructor’s view contents are different, I would like to start to investigate the db first.
If you know any related java classes, please let us know that too.
Thank you in advance.

Sanghyun





-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://collab.sakaiproject.org/pipermail/sakai-dev/attachments/20110224/93622400/attachment.html