chore: make function get_latest and get_best_submissions reusable for both student and instructor version

#250

Author: mpetojevic

grader_service/handlers/base_handler.py

@@ -51,6 +51,7 @@
 
 auth_header_pat = re.compile(r'^(token|bearer|basic)\s+([^\s]+)$', flags=re.IGNORECASE)
 
+
 def check_authorization(self: "GraderBaseHandler", scopes: list[Scope], lecture_id: Union[int, None]) -> bool:
     if (("/permissions" in self.request.path)
             or ("/config" in self.request.path)):
@@ -112,6 +113,7 @@ async def request_handler_wrapper(self: "GraderBaseHandler", *args,
 
     return wrapper
 
+
 class BaseHandler(web.RequestHandler):
     """Base class of all handler classes
 
@@ -136,13 +138,13 @@ def __init__(
         self.log = self.application.log
 
     async def prepare(self) -> Optional[Awaitable[None]]:
-        #strip trailing slash
+        # strip trailing slash
         self.request.path = self.request.path.rstrip("/")
-        
-        #start session
+
+        # start session
         self.session: Session = self.application.session_maker()
-        
-        #authenticate
+
+        # authenticate
         try:
             await self.get_current_user()
 
@@ -154,25 +156,27 @@ async def prepare(self) -> Optional[Awaitable[None]]:
                 url_path_join(self.application.base_url, "/api/oauth2/token"),
                 url_path_join(self.application.base_url, "/oauth_callback"),
                 url_path_join(self.application.base_url, "/lti13/oauth_callback")
-                ]:
+            ]:
                 # require git to authenticate with token -> otherwise return 401 code
                 if self.request.path.startswith(url_path_join(self.application.base_url, "/git")):
                     raise HTTPError(401, reason="Git: authenticate request")
-                
+
                 # send to login page if ui page request
-                if self.request.path in [url_path_join(self.application.base_url, "/api/oauth2/authorize")] or self.request.path.startswith(url_path_join(self.application.base_url, "/ui")):
+                if self.request.path in [
+                    url_path_join(self.application.base_url, "/api/oauth2/authorize")] or self.request.path.startswith(
+                    url_path_join(self.application.base_url, "/ui")):
                     url = url_concat(self.settings["login_url"], dict(next=self.request.uri))
                     self.redirect(url)
                     return
-                    
+
                 if self.request.headers.get("Authorization") is None:
                     raise HTTPError(401, reason="No API token in auth header")
-                    
+
                 # do not redirect to login page if we hit api endpoints
                 raise HTTPError(401, reason="API Token is invalid or expired.")
-                
-                
-                
+
+
+
         except Exception as e:
             # ensure get_current_user is never called again for this handler,
             # since it failed
@@ -775,7 +779,7 @@ def append_query_parameters(self, url, exclude=None):
 
 
 class GraderBaseHandler(BaseHandler):
-        
+
     def validate_parameters(self, *args):
         if len(self.request.arguments) == 0:
             return
@@ -816,56 +820,65 @@ def get_submission(self, lecture_id: int, assignment_id: int,
             raise HTTPError(HTTPStatus.NOT_FOUND,
                             reason=msg)
         return submission
-    
-    def get_latest_submissions(self, assignment_id, must_have_feedback = False):
+
+    def get_latest_submissions(self, assignment_id, must_have_feedback=False, username=None):
         subquery = (
             self.session.query(Submission.username,
-                                func.max(Submission.date).label(
-                                    "max_date"))
+                               func.max(Submission.date).label("max_date"))
             .filter(Submission.assignid == assignment_id)
             .filter(Submission.deleted == DeleteState.active)
             .group_by(Submission.username)
             .subquery())
+
         if must_have_feedback:
             subquery = subquery.filter(Submission.feedback_status != "not_generated")
 
-        # build the main query
+        if username:
+            subquery = subquery.filter(Submission.username == username)
+
+        # Build the main query
         submissions = (
             self.session.query(Submission)
             .join(subquery,
-                    (Submission.username == subquery.c.username) & (
-                            Submission.date == subquery.c.max_date) & (
-                            Submission.assignid == assignment_id) & (
-                            Submission.deleted == DeleteState.active
-                            ))
+                  (Submission.username == subquery.c.username) & (
+                          Submission.date == subquery.c.max_date) & (
+                          Submission.assignid == assignment_id) & (
+                          Submission.deleted == DeleteState.active
+                  ))
             .order_by(Submission.id)
-            .all())
+            .all()
+        )
+
         return submissions
-    
-    def get_best_submissions(self, assignment_id, must_have_feedback = False):
-        # build the subquery
-        subquery = (self.session.query(Submission.username, func.max(
-            Submission.score).label("max_score"))
-                    .filter(Submission.assignid == assignment_id)
-                    .filter(Submission.deleted == DeleteState.active)
-                    .group_by(Submission.username)
-                    .subquery())
-        
+
+    def get_best_submissions(self, assignment_id, must_have_feedback=False, username=None):
+        subquery = (
+            self.session.query(Submission.username,
+                               func.max(Submission.score).label("max_score"))
+            .filter(Submission.assignid == assignment_id)
+            .filter(Submission.deleted == DeleteState.active)
+            .group_by(Submission.username)
+            .subquery())
+
         if must_have_feedback:
             subquery = subquery.filter(Submission.feedback_status != "not_generated")
 
-        # build the main query
+        if username:
+            subquery = subquery.filter(Submission.username == username)
+
+        # Build the main query
         submissions = (
             self.session.query(Submission)
             .join(subquery,
-                    (Submission.username == subquery.c.username) & (
-                            Submission.score == subquery.c.max_score) & (
-                            Submission.assignid == assignment_id) & (
-                            Submission.deleted == DeleteState.active
-                            ))
+                  (Submission.username == subquery.c.username) & (
+                          Submission.score == subquery.c.max_score) & (
+                          Submission.assignid == assignment_id) & (
+                          Submission.deleted == DeleteState.active
+                  ))
             .group_by(Submission.username)
             .order_by(Submission.id)
-            .all())
+            .all()
+        )
         return submissions
 
     @property

grader_service/handlers/submissions.py

@@ -9,6 +9,7 @@
 
 from sqlalchemy import label
 
+from grader_service.api.models import Lecture
 from grader_service.orm.base import DeleteState
 import isodate
 import os.path
@@ -173,83 +174,24 @@ async def get(self, lecture_id: int, assignment_id: int):
         role: Role = self.get_role(lecture_id)
         if instr_version and role.role < Scope.tutor:
             raise HTTPError(HTTPStatus.FORBIDDEN, reason="Forbidden")
-        assignment = self.get_assignment(lecture_id, assignment_id)
 
-        if instr_version:
-            if submission_filter == 'latest':
-                submissions = self.get_latest_submissions(assignment_id)
-            elif submission_filter == 'best':
-                submissions = self.get_best_submissions(assignment_id)
-            else:
-                submissions = [
-                s for s in assignment.submissions if (s.deleted
-                                                        == DeleteState.active)
-                ]
+        username = None if instr_version else role.username
+        if submission_filter == "latest":
+            submissions = self.get_latest_submissions(
+                assignment_id, username=username)
+        elif submission_filter == "best":
+            submissions = self.get_best_submissions(
+                assignment_id, username=username)
         else:
-            if submission_filter == 'latest':
-                # build the subquery
-                subquery = (self.session.query(Submission.username,
-                                               func.max(Submission.date).label(
-                                                   "max_date"))
-                            .filter(Submission.assignid == assignment_id)
-                            .filter(Submission.deleted == DeleteState.active)
-                            .group_by(Submission.username)
-                            .subquery())
-
-                # build the main query
-                submissions = (
-                    self.session.query(Submission)
-                    .join(subquery,
-                          (Submission.username == subquery.c.username) & (
-                                  Submission.date == subquery.c.max_date) & (
-                                  Submission.assignid == assignment_id) & (
-                                  Submission.deleted == DeleteState.active
-                                  ))
-                    .filter(
-                        Submission.assignid == assignment_id,
-                        Submission.username == role.username, )
-                    .order_by(Submission.id)
-                    .all())
-
-            elif submission_filter == 'best':
-
-                # build the subquery
-                subquery = (self.session.query(Submission.username, func.max(
-                    Submission.score).label("max_score"))
-                            .filter(Submission.assignid == assignment_id)
-                            .filter(Submission.deleted == DeleteState.active)
-                            .group_by(Submission.username)
-                            .subquery())
-
-                # build the main query
-                submissions = (
-                    self.session.query(Submission)
-                    .join(subquery,
-                          (Submission.username == subquery.c.username) & (
-                                  Submission.score == subquery.c.max_score) & (
-                                  Submission.assignid == assignment_id) & (
-                                  Submission.deleted == DeleteState.active
-                                  ))
-                    .filter(
-                        Submission.assignid == assignment_id,
-                        Submission.username == role.username, )
-                    .order_by(Submission.id)
-                    .all())
-            else:
-                submissions = (
-                    self.session.query(
-                        Submission
-                    )
-                    .filter(
-                        Submission.assignid == assignment_id,
-                        Submission.username == role.username,
-                        Submission.deleted == DeleteState.active
-                    )
-                    .order_by(Submission.id)
-                    .all()
-                )
+            query = self.session.query(Submission).filter(
+                Submission.assignid == assignment_id,
+                Submission.deleted == DeleteState.active
+            )
+            if username:
+                query = query.filter(Submission.username == username)
+            submissions = query.order_by(Submission.id).all()
+
         if response_format == "csv":
-            # csv format does not include logs
             self.set_header("Content-Type", "text/csv")
             for i, s in enumerate(submissions):
                 d = s.model.to_dict()
@@ -263,7 +205,7 @@ async def get(self, lecture_id: int, assignment_id: int):
                 submissions = remove_points_from_submission(submissions)
 
             self.write_json(submissions)
-        self.session.close()  # manually close here because on_finish overwrite
+        self.session.close()
 
     @authorize([Scope.student, Scope.tutor, Scope.instructor])
     async def post(self, lecture_id: int, assignment_id: int):