Skip to content

Commit

Permalink
Move helper functions to DashboardService
Browse files Browse the repository at this point in the history
This will enable:

- Tackle the complexity of checking permission for different objects in a
  more standard way.
- Move the API dashboard API endpoints into view/api while still be able
  to share this code with the other dashboard views.
  • Loading branch information
marcospri committed Jul 3, 2024
1 parent abdc9d4 commit c9e0bb5
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 133 deletions.
3 changes: 3 additions & 0 deletions lms/services/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from lms.services.aes import AESService
from lms.services.application_instance import ApplicationInstanceNotFound
from lms.services.assignment import AssignmentService
from lms.services.canvas import CanvasService
from lms.services.canvas_studio import CanvasStudioService
from lms.services.d2l_api.client import D2LAPIClient
Expand Down Expand Up @@ -136,6 +137,8 @@ def includeme(config):
config.register_service_factory(
"lms.services.d2l_api.d2l_api_client_factory", iface=D2LAPIClient
)
config.register_service_factory("lms.services.dashboard.factory", name="dashboard")

config.register_service_factory(
"lms.services.digest.service_factory", iface=DigestService
)
Expand Down
68 changes: 68 additions & 0 deletions lms/services/dashboard.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
from pyramid.httpexceptions import HTTPNotFound, HTTPUnauthorized

from lms.security import Permissions
from lms.services.organization import OrganizationService


class DashboardService:
def __init__(self, assignment_service, course_service, organization_service):
self._assignment_service = assignment_service
self._course_service = course_service
self._organization_service = organization_service

def get_request_assignment(self, request):
"""Get and authorize an assignment for the given request."""
assignment = self._assignment_service.get_by_id(
request.matchdict["assignment_id"]
)
if not assignment:
raise HTTPNotFound()

if request.has_permission(Permissions.STAFF):
# STAFF members in our admin pages can access all assignments
return assignment

if not self._assignment_service.is_member(assignment, request.user.h_userid):
raise HTTPUnauthorized()

return assignment

def get_request_course(self, request):
"""Get and authorize a course for the given request."""
course = self._course_service.get_by_id(request.matchdict["course_id"])
if not course:
raise HTTPNotFound()

if request.has_permission(Permissions.STAFF):
# STAFF members in our admin pages can access all courses
return course

if not self._course_service.is_member(course, request.user.h_userid):
raise HTTPUnauthorized()

return course

def get_request_organization(self, request):
"""Get and authorize an organization for the given request."""
organization = self._organization_service.get_by_public_id(
public_id=request.matchdict["organization_public_id"]
)
if not organization:
raise HTTPNotFound()

if request.has_permission(Permissions.STAFF):
# STAFF members in our admin pages can access all organizations
return organization

if not self._organization_service.is_member(organization, request.user):
raise HTTPUnauthorized()

return organization


def factory(_context, request):
return DashboardService(
assignment_service=request.find_service(name="assignment"),
course_service=request.find_service(name="course"),
organization_service=request.find_service(OrganizationService),
)
6 changes: 3 additions & 3 deletions lms/views/dashboard/api/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from lms.models import Assignment, RoleScope, RoleType
from lms.security import Permissions
from lms.services.h_api import HAPI
from lms.views.dashboard.base import get_request_assignment
from lms.views.dashboard.pagination import PaginationParametersMixin, get_page


Expand All @@ -28,6 +27,7 @@ def __init__(self, request) -> None:
self.request = request
self.h_api = request.find_service(HAPI)
self.assignment_service = request.find_service(name="assignment")
self.dashboard_service = request.find_service(name="dashboard")

@view_config(
route_name="api.dashboard.assignments",
Expand Down Expand Up @@ -59,7 +59,7 @@ def assignments(self) -> APIAssignments:
permission=Permissions.DASHBOARD_VIEW,
)
def assignment(self) -> APIAssignment:
assignment = get_request_assignment(self.request, self.assignment_service)
assignment = self.dashboard_service.get_request_assignment(self.request)
return APIAssignment(
id=assignment.id,
title=assignment.title,
Expand All @@ -74,7 +74,7 @@ def assignment(self) -> APIAssignment:
)
def assignment_stats(self) -> APIStudents:
"""Fetch the stats for one particular assignment."""
assignment = get_request_assignment(self.request, self.assignment_service)
assignment = self.dashboard_service.get_request_assignment(self.request)
stats = self.h_api.get_annotation_counts(
[g.authority_provided_id for g in assignment.groupings],
group_by="user",
Expand Down
8 changes: 4 additions & 4 deletions lms/views/dashboard/api/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from lms.security import Permissions
from lms.services.h_api import HAPI
from lms.services.organization import OrganizationService
from lms.views.dashboard.base import get_request_course, get_request_organization
from lms.views.dashboard.pagination import PaginationParametersMixin, get_page

MAX_ITEMS_PER_PAGE = 100
Expand All @@ -32,6 +31,7 @@ def __init__(self, request) -> None:
self.course_service = request.find_service(name="course")
self.h_api = request.find_service(HAPI)
self.organization_service = request.find_service(OrganizationService)
self.dashboard_service = request.find_service(name="dashboard")

@view_config(
route_name="api.dashboard.courses",
Expand Down Expand Up @@ -61,7 +61,7 @@ def courses(self) -> APICourses:
permission=Permissions.DASHBOARD_VIEW,
)
def organization_courses(self) -> APICourses:
org = get_request_organization(self.request, self.organization_service)
org = self.dashboard_service.get_request_organization(self.request)
courses = self.request.db.scalars(
self.course_service.get_courses(
organization=org,
Expand Down Expand Up @@ -93,7 +93,7 @@ def organization_courses(self) -> APICourses:
permission=Permissions.DASHBOARD_VIEW,
)
def course(self) -> APICourse:
course = get_request_course(self.request, self.course_service)
course = self.dashboard_service.get_request_course(self.request)
return {
"id": course.id,
"title": course.lms_name,
Expand All @@ -106,7 +106,7 @@ def course(self) -> APICourse:
permission=Permissions.DASHBOARD_VIEW,
)
def course_assignments(self) -> APIAssignments:
course = get_request_course(self.request, self.course_service)
course = self.dashboard_service.get_request_course(self.request)
course_students = self.course_service.get_members(
course, role_scope=RoleScope.COURSE, role_type=RoleType.LEARNER
)
Expand Down
50 changes: 0 additions & 50 deletions lms/views/dashboard/base.py

This file was deleted.

12 changes: 4 additions & 8 deletions lms/views/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
from lms.security import Permissions
from lms.services import OrganizationService
from lms.validation.authentication import BearerTokenSchema
from lms.views.dashboard.base import (
get_request_assignment,
get_request_course,
get_request_organization,
)


@forbidden_view_config(
Expand Down Expand Up @@ -43,6 +38,7 @@ def __init__(self, request) -> None:
self.organization_service: OrganizationService = request.find_service(
OrganizationService
)
self.dashboard_service = request.find_service(name="dashboard")

@view_config(
route_name="dashboard.launch.assignment",
Expand Down Expand Up @@ -77,7 +73,7 @@ def assignment_show(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
assignment = get_request_assignment(self.request, self.assignment_service)
assignment = self.dashboard_service.get_request_assignment(self.request)
self.request.context.js_config.enable_dashboard_mode()
self._set_lti_user_cookie(self.request.response)
return {"title": assignment.title}
Expand All @@ -93,7 +89,7 @@ def course_show(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
course = get_request_course(self.request, self.course_service)
course = self.dashboard_service.get_request_course(self.request)
self.request.context.js_config.enable_dashboard_mode()
self._set_lti_user_cookie(self.request.response)
return {"title": course.lms_name}
Expand All @@ -110,7 +106,7 @@ def organization_show(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
# Just get the org for the side effect of checking permissions.
_ = get_request_organization(self.request, self.organization_service)
_ = self.dashboard_service.get_request_organization(self.request)
self.request.context.js_config.enable_dashboard_mode()
self._set_lti_user_cookie(self.request.response)
# Org names are not 100% ready for public consumption, let's hardcode a title for now.
Expand Down
Loading

0 comments on commit c9e0bb5

Please sign in to comment.