Skip to content

issue_1671 the first attempt is to capture the sql error and display the error in grade distribution view UI #1674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zqian wants to merge 2 commits into
base: master
Choose a base branch
from
Open

issue_1671 the first attempt is to capture the sql error and display the error in grade distribution view UI #1674

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fcb6b4
issue_1671 the first attempt is to capture the sql error and display …
zqian Sep 16, 2025
dad2841
issue_1671 log error message based on the Codacy review
zqian Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions dashboard/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import jsonschema
import pandas as pd
from sqlalchemy.exc import SQLAlchemyError
from constance import config
from django.conf import settings
from django.contrib import auth
Expand Down Expand Up @@ -460,11 +461,18 @@ def grade_distribution(request, course_id=0):
(select current_grade from user where sis_name=%(current_user)s and course_id=%(course_id)s) as current_user_grade
from user where course_id=%(course_id)s and enrollment_type=%(enrollment_type)s
"""
df = pd.read_sql(grade_score_sql, app_engine, params={
'current_user': current_user,
'course_id': course_id,
'enrollment_type': 'StudentEnrollment'
})
try:
logger.info(f"course_id={course_id}, current_user={current_user}")
df = pd.read_sql(grade_score_sql, app_engine, params={
'current_user': current_user,
'course_id': course_id,
'enrollment_type': 'StudentEnrollment'
})
except SQLAlchemyError as e:
grade_distribution_sql_error_msg = f'Error running grade distribution sql with course_id={course_id}, current_user={current_user} enrollment_type=StudentEnrollment'
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing comma between '{current_user}' and 'enrollment_type'. Should be 'current_user={current_user}, enrollment_type=StudentEnrollment'.

Suggested change
grade_distribution_sql_error_msg = f'Error running grade distribution sql with course_id={course_id}, current_user={current_user} enrollment_type=StudentEnrollment'
grade_distribution_sql_error_msg = f'Error running grade distribution sql with course_id={course_id}, current_user={current_user}, enrollment_type=StudentEnrollment'

Copilot uses AI. Check for mistakes.
logger.error(f"{grade_distribution_sql_error_msg} sql={grade_score_sql} error={e}")
return HttpResponse(json.dumps({'gd_disable':'true','gd_msg': grade_distribution_sql_error_msg}), content_type='application/json')
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use JsonResponse instead of manually constructing JSON with HttpResponse and json.dumps. This provides better consistency with Django conventions and automatically sets the content type.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boolean value 'true' is returned as a string. Consider using a boolean type (True) in the dictionary for consistency, as json.dumps will correctly serialize it to the JSON boolean 'true'.

Copilot uses AI. Check for mistakes.

if len(df) <= config.GRADE_DISTRIBUTION_MINIMUM:
grade_distribution_limit_msg = f'Grade Distribution view is disabled because the course enrollment is less than {config.GRADE_DISTRIBUTION_MINIMUM}'
logger.error(f"Course enrollment count {len(df)} Hence the {grade_distribution_limit_msg}")
Expand Down