Skip to content

Commit

Permalink
Don't include public_id on the dashbaord config when not present
Browse files Browse the repository at this point in the history
This way we avoid having to deal with undefined vs null values in the
frontend.
  • Loading branch information
marcospri committed Aug 1, 2024
1 parent afc52a3 commit 6b38fb5
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lms/js_config_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class User(TypedDict):
class DashboardConfig(TypedDict):
user: User

organization_public_id: str
organization_public_id: NotRequired[str]
"""
Filtering by organization is not like other filters:
- It's only available to staff members
Expand Down
3 changes: 2 additions & 1 deletion lms/resources/_js_config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ def enable_dashboard_mode(self) -> None:
"mode": JSConfig.Mode.DASHBOARD,
"dashboard": DashboardConfig(
user=self._get_user_info(),
organization_public_id=self._request.params.get("public_id"),
routes=DashboardRoutes(
assignment=self._to_frontend_template(
"api.dashboard.assignment"
Expand All @@ -281,6 +280,8 @@ def enable_dashboard_mode(self) -> None:
),
}
)
if organization_public_id := self._request.params.get("public_id"):
self._config["dashboard"]["organization_public_id"] = organization_public_id

def enable_lti_launch_mode(self, course, assignment: Assignment):
"""
Expand Down
14 changes: 9 additions & 5 deletions tests/unit/lms/resources/_js_config/__init___test.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,17 +694,13 @@ def LTIEvent(self, patch):


class TestEnableDashboardMode:
@pytest.mark.parametrize("public_id", ["PUBLIC_ID", None])
def test_it(self, js_config, lti_user, pyramid_request, public_id):
pyramid_request.params["public_id"] = public_id

def test_it(self, js_config, lti_user):
js_config.enable_dashboard_mode()
config = js_config.asdict()

assert config["mode"] == JSConfig.Mode.DASHBOARD
assert config["dashboard"] == {
"user": {"display_name": lti_user.display_name, "is_staff": False},
"organization_public_id": public_id,
"routes": {
"assignment": "/api/dashboard/assignments/:assignment_id",
"students_metrics": "/api/dashboard/students/metrics",
Expand All @@ -717,6 +713,14 @@ def test_it(self, js_config, lti_user, pyramid_request, public_id):
},
}

def test_it_when_organizataion_public_id_present(self, js_config, pyramid_request):
pyramid_request.params["public_id"] = sentinel.public_id

js_config.enable_dashboard_mode()
config = js_config.asdict()

assert config["dashboard"]["organization_public_id"] == sentinel.public_id

def test_user_when_staff(self, js_config, pyramid_request_staff_member, context):
js_config = JSConfig(context, pyramid_request_staff_member)
js_config.enable_dashboard_mode()
Expand Down

0 comments on commit 6b38fb5

Please sign in to comment.