Conversation
BiancaNecula
added
enhancement
Codecov Report
@@ Coverage Diff @@
## master #256 +/- ##
==========================================
+ Coverage 86.08% 86.51% +0.42%
==========================================
Files 19 19
Lines 891 949 +58
Branches 77 84 +7
==========================================
+ Hits 767 821 +54
- Misses 105 106 +1
- Partials 19 22 +3
Continue to review full report at Codecov.
|
interface/scoring.py
Outdated
| for comment in submission.comments.all(): | ||
| if ( | ||
| comment.user | ||
| in submission.assignment.course.teaching_assistants.all() |
There was a problem hiding this comment.
Q: Could you save this variable before the outer for? and then do only
if comment.user in teaching_assistants:
| <div class="col-lg-2"></div> | ||
| <div class="col-lg-9"> | ||
| <pre> | ||
| /^--^\ /^--^\ /^--^\ |
There was a problem hiding this comment.
. .
|\_---_/|
/ o_o \
| U |
\ ._I_. /
`-_____-'
| @@ -163,11 +163,11 @@ def test_user_cannot_review(client, STC, base_db_setup): | |||
|
|
|||
| review_message = "+10.0: Hacker" | |||
| response = client.post( | |||
There was a problem hiding this comment.
Q: Could you also put a test where the student comments with something like? 🙏
+10.0: I really did a nice job!
interface/views.py
Outdated
| } | ||
|
|
||
| if ( | ||
| file.getvalue() != "The file is missing!" |
There was a problem hiding this comment.
Q: Could you save that value to don't having to call the same method twice?
like
file_value = file.getValue()
and then you can do
if file_value not in ["The file is missing!", "The archive is missing!"]:
interface/views.py
Outdated
|
|
||
| tree = tree_view(request, submission) | ||
|
|
||
| if filename == "code": |
There was a problem hiding this comment.
This might introduce the issue of actually having a file named code in the archive. A solution could be having two paths in our urlpatterns:
"submission/<int:pk>/code/",
"submission/<int:pk>/code/<path:filename>/",
Those two should point to different views. One will handle the if case and the other the else case. It will also make it clearer where you are in your logic (empty page / page with code).
| comment = form.save(commit=False) | ||
| comment.submission = submission | ||
| comment.user = request.user | ||
| comment.path = filename | ||
| comment.line = request.POST.get("line", "") | ||
| comment.save() |
There was a problem hiding this comment.
Maybe you can add hidden inputs in your form so those can be filled in by form.save().
| all_tas = submission.assignment.course.teaching_assistants.all() | ||
|
|
||
| if request.user not in all_tas and request.user is not submission.user: | ||
| return HttpResponse(status=403) |
There was a problem hiding this comment.
Q: Could you also add a test for this? 👍
3 participants
Description
- This feature allows the ta to review and grade the homework directly on the code. They can leave multiple comments on every line (which will be highlighted) and change the score. The student can reply back.
Fixes #165
Testing
Checklist