228 Approve blocked submissions

[webecke]

This PR will build off of the work that is being done in #233

Once the code written in #233 has marked the new approved variable as false on a submission, this PR will do the following:

• Mark the submission in the admin view as needing approval (both in the submissions tab, and on the student's info box)
• Provide an interface for any admin to approve the submission and send the grade to canvas
  • They will have the option to approve with penalty (currently set to 10%)
  • Or to approve without penalty (meaning they get full points)
• Notify the autograder server of the admin's decision
• Update the autograder database of the approved status and score
• Send the score to canvas with a note

Database Modifications

Run the following SQL commands to configure the database with the needed values:

INSERT INTO autograder.rubric_config (phase, type, category, criteria, points) VALUES ('Commits', 'GIT_COMMITS', 'Git Commits', 'At least 10 GitHub commits evenly spread over the assignment period that demonstrate proof of work.', 0);

[frozenfrank]

@DallinFromEarth What's the status on this PR? I see it's marked as a "Draft." My code is ready for you to start integrating it. I defined the behavior, and implemented the DAO and service level methods you'll need to create the proper endpoints.

In the end, the fields a richer than a simple boolean value, but they'll be simple to work with anyways.

How can we collaborate to finish both of these and get this new feature into the product? (See #233)

[frozenfrank]

This is coming along really well! If we're both railroad companies approaching each other from separate coasts, we're within smelling distance of nailing down the golden spike! 🚂

I've left a few comments about how we'll need to connect up exactly. I'm hoping that we can talk more about this tomorrow morning during our shared TA hours.

At any rate, we'll probably end up merging one of these PRs into the other to finish up the work, and then submitting it all as PR into main. How do you feel about that?

[webecke]

@frozenfrank The UI stuff has been done for a while now. Now that you've completed all this amazing work, I can go ahead and work on connecting it. So far this morning I worked on getting main and your branch merged into mine so I can be working on the right code. Once I finish looking through your additions I'll have a better idea of where I need to go

[webecke]

@frozenfrank What updates did you make to the database? I've noticed the new verified_status and verification columns, but I apparently can't quite figure out the SQL statement I need to run to get it to work

[frozenfrank]

@frozenfrank What updates did you make to the database? I've noticed the new verified_status and verification columns, but I apparently can't quite figure out the SQL statement I need to run to get it to work

Great question, @DallinFromEarth!

The answer to this is too long to type out, but I recorded two videos going over the details for you:

• Changes to SQL DAO
• Submission Approval DAO methods 1
• Submission Approval DAO methods 2

[webecke]

I did see the updates you described in the video. They look great, and I think they'll make life easier.

The issue I was having was when running the code and going to the front end, it was throwing SQL access error, which in my experience normally happened when the database was changed and the columns weren't added to my database. I then ran ALTER TABLE submission ADD COLUMN verified_status VARCHAR(255), ADD COLUMN verification VARCHAR(255) which I had thought would fix it. It did not.

This took me a lot longer than I care to admit to figure out, but the issue was in getAllLatestSubmissions, the code was passing in a statement with selectAllStmt already prepended, then the executeQuery method was adding adding again, causing the SQL statement to look like this:

SELECT net_id, repo_url, timestamp, phase, passed, score, head_hash, notes, rubric, admin, verified_status, verification 
FROM submission 
SELECT net_id, repo_url, timestamp, phase, passed, score, head_hash, notes, rubric, admin, verified_status, verification 
FROM submission 
WHERE timestamp IN (
    SELECT MAX(timestamp)
    FROM submission
    GROUP BY net_id, phase
)
ORDER BY timestamp DESC
LIMIT ?

That was throwing the error. I've fixed it in my branch

[frozenfrank]

@DallinFromEarth

... The issue was in getAllLatestSubmissions, the code was passing in a statement with selectAllStmt already prepended, then the executeQuery method was adding adding again.

Yes, unfortunately, that was a bug in my code. Sorry about that. I also discovered that issue and resolved it in commit ad67936 which has been added to the 281-backend-streamline-sql-dao-column-list-management branch tracked by #284. You can re-merge in that branch and it should also resolve the issue.

🐑 😞 🐑

[frozenfrank]

@DallinFromEarth Also note that I pushed a few commits to the #233 PR which allow the checks (tests) to pass again. Merging in those changes will likely cause this PR to pass the checks again as well.

[webecke]

Hey @frozenfrank I merged the branches you've mentioned and have gotten kind of stuck. I might have done something wrong, but I can't get anything to run properly. Everything keeps running into SQL errors. I've spent a long time today trying to hunt them down. Can we talk sometime tomorrow or Friday?

One example is, at least on my machine, that the insert builder doesn't work. INSERT INTO queue (net_id, phase, started, time_added) VALUES (?, ?, ?, ?,). At least on Mac, that trailing comma is fatal. I fixed it on my branch.

Another issue I'm running into is how to handle first time submissions. Currently, if this is the student's first submission, then it will fail because it doesn't have an upper bound submission hash, and throws an error.

[frozenfrank]

I appreciate your code, @DallinFromEarth. I'm "reviewing" it so I can keep up on how we are integrating together.

I apologize that my code is giving you so many issues. I feel bad that you have to struggle through the bugs I wrote 🐑

Can you push any other changes you may have to this branch? Or, push them directly to the 281-... branch, if they're appropriate?

[frozenfrank]

Hey @frozenfrank I merged the branches you've mentioned and have gotten kind of stuck. I might have done something wrong, but I can't get anything to run properly. Everything keeps running into SQL errors. I've spent a long time today trying to hunt them down. Can we talk sometime tomorrow or Friday?

Let's definitely talk tomorrow morning. We can figure it out together!

One example is, at least on my machine, that the insert builder doesn't work. INSERT INTO queue (net_id, phase, started, time_added) VALUES (?, ?, ?, ?,). At least on Mac, that trailing comma is fatal. I fixed it on my branch.

💀 Rip me. I had specifically searched to figure out if the trailing comma would be acceptable, but I guess it wasn't. Maybe when we work together, I can become more comfortable running the code to see it actually work. Up to this point, I've been mostly solving the problem theoretically which has been forcing you to deal with the consequences.

Can you push the fix directly to #284 though?

Another issue I'm running into is how to handle first time submissions. Currently, if this is the student's first submission, then it will fail because it doesn't have an upper bound submission hash, and throws an error.

Interesting. I had thought through those details, but I probably made a mistake somewhere. I'm surprised that it's complaining about the "upper bound submission hash" because that's supposed to be the hash of the current submission. I would have expected this issue to result in a missing "lower bound submission hash." We'll be able to solve this together in the morning.

[frozenfrank]

One example is, at least on my machine, that the insert builder doesn't work. INSERT INTO queue (net_id, phase, started, time_added) VALUES (?, ?, ?, ?,). At least on Mac, that trailing comma is fatal. I fixed it on my branch.

Another issue I'm running into is how to handle first time submissions. Currently, if this is the student's first submission, then it will fail because it doesn't have an upper bound submission hash, and throws an error.

Both of these issues have been resolved in new commits on the #233 PR. @DallinFromEarth

[frozenfrank]

This is now ready for review!

The Canvas integration initially attempting to be built out independent in this branch, has been ripped out and converted instead to existing Canvas score methods in the application. Those methods had to be adapted to allow this to occur. For a review of how they were adjusted, read the commit history commit-by-commit as they record the changes.

This PR now approves student's on a phase, and continues applying the appropriate penalty on future submissions.

Screenshots

@19mdavenport

[19mdavenport]

Grades are not correctly being sent to canvas when a TA approves the submission. The rubric items for test cases/code quality aren't included. If the submission is approved with a penalty, only the git commits item gets sent (as 0/0 since everything else is 0 points). If the submission is approved without a penalty, either nothing is sent to canvas, or canvas isn't receiving anything of note and doesn't display it

Good job with the approved value being applied to future submissions.

Looks like the merge may not have been 100% successful. GitHelper lines 160-165 were removed in a different branch. Would you be ok removing those so students don't see the commit verification pass/failure until after they pass the test cases?

[19mdavenport]

Ok I think I fixed the problems with scores not being sent successfully. @Fiwafoofa (and @frozenfrank if you're still interested) can you go over this and see if you see any other potential problems?

[Fiwafoofa]

It looks like it works! A slight inconsistency I noticed is that when a student receives a penalty, they are notified that It will come with a 10% penalty. so I updated the 'will' to 'may'. Otherwise, it looks good to me.

stale