-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
Query Limits Management Feature #7
base: main
Are you sure you want to change the base?
Changes from all commits
44476c6
48c4d77
4ddfa12
9506cd7
8937e0c
53fdce7
fc344cc
f58a46e
506de16
2871915
b259224
e2925dc
fc00b67
f346e43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,4 +14,4 @@ venv/ | |
|
||
*.egg-info/ | ||
|
||
.aider* | ||
.aider* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
-- SPDX-FileCopyrightText: Mark Liffiton <[email protected]>, Rana Moeez Hassan | ||
-- | ||
-- SPDX-License-Identifier: AGPL-3.0-only | ||
|
||
BEGIN; | ||
|
||
-- Add columns to the classes table | ||
ALTER TABLE classes ADD COLUMN query_limit_enabled BOOLEAN NOT NULL DEFAULT 0; | ||
ALTER TABLE classes ADD COLUMN max_queries INTEGER NOT NULL DEFAULT 50; | ||
|
||
-- Ensure the users table has a column to track the number of queries used | ||
ALTER TABLE users ADD COLUMN queries_used INTEGER NOT NULL DEFAULT 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is just tied to the user, then it won't work correctly for users in multiple classes, right? I think this might need a different design. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I did not realize that a student account could be in multiple classes. Will need to rethink this. |
||
|
||
COMMIT; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,9 @@ | |
from .csv import csv_response | ||
from .data_deletion import delete_class_data | ||
from .db import get_db | ||
from .auth import get_auth | ||
from .redir import safe_redirect | ||
from .tables import BoolCol, DataTable, NumCol, UserCol | ||
from .tables import ButtonCol, BoolCol, Col, DataTable, NumCol, UserCol | ||
|
||
bp = Blueprint('instructor', __name__, template_folder='templates') | ||
|
||
|
@@ -85,6 +86,32 @@ def _get_class_users(*, for_export: bool = False) -> list[Row]: | |
|
||
return users | ||
|
||
def _get_query_limits_data() -> list[Row]: | ||
cur_class = get_auth_class() | ||
class_id = cur_class.class_id | ||
db = get_db() | ||
|
||
users = db.execute(""" | ||
SELECT | ||
users.id, | ||
json_array(users.display_name, auth_providers.name, users.display_extra) AS user, | ||
users.queries_used, | ||
classes.max_queries, | ||
('<form method="POST" action="/instructor/reset_student_queries/' || users.id || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generating HTML in the SQL statement is an overly-complex way to accomplish this. I think using the |
||
'" style="display:inline"><button class="button is-small is-warning" type="submit">' || | ||
'<span class="icon"><img src="data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIyNCIgaGVpZ2h0PSIyNCIgdmlld0JveD0iMCAwIDI0IDI0IiBmaWxsPSJub25lIiBzdHJva2U9ImN1cnJlbnRDb2xvciIgc3Ryb2tlLXdpZHRoPSIyIiBzdHJva2UtbGluZWNhcD0icm91bmQiIHN0cm9rZS1saW5lam9pbj0icm91bmQiIGNsYXNzPSJsdWNpZGUgbHVjaWRlLWEtYXJyb3ctZG93biI+PHBhdGggZD0iTTMuNSAxM2g2Ii8+PHBhdGggZD0ibTIgMTYgNC41LTkgNC41IDkiLz48cGF0aCBkPSJNMTggN3Y5Ii8+PHBhdGggZD0ibTE0IDEyIDQgNCA0LTQiLz48L3N2Zz4=" alt="reset icon" style="width: 1em; height: 1em;"></span>' || | ||
'<span>Reset</span></button></form>') AS reset_button | ||
|
||
FROM users | ||
LEFT JOIN auth_providers ON users.auth_provider=auth_providers.id | ||
JOIN roles ON roles.user_id=users.id | ||
JOIN classes ON roles.class_id=classes.id | ||
WHERE roles.class_id=? AND roles.role='student' | ||
ORDER BY users.display_name | ||
""", [class_id]).fetchall() | ||
|
||
return users | ||
|
||
|
||
@bp.route("/") | ||
def main() -> str | Response: | ||
|
@@ -98,6 +125,12 @@ def main() -> str | Response: | |
data=users, | ||
) | ||
|
||
queries_limits_table = DataTable( | ||
name='query_limits', | ||
columns=[NumCol('id', hidden=True), UserCol('user'), NumCol('queries_used', align="center"), NumCol('max_queries', align="center"), Col('reset_button', kind='html', align="center")], | ||
data=_get_query_limits_data() | ||
) | ||
|
||
sel_user_name = None | ||
sel_user_id = request.args.get('user', type=int) | ||
if sel_user_id is not None: | ||
|
@@ -110,7 +143,8 @@ def main() -> str | Response: | |
queries_table.data = _get_class_queries(sel_user_id) | ||
queries_table.csv_link = url_for('instructor.get_csv', kind='queries') | ||
|
||
return render_template("instructor_view.html", users=users_table, queries=queries_table, user=sel_user_name) | ||
return render_template( "instructor_view.html", users=users_table, queries=queries_table, query_limits=queries_limits_table, user=sel_user_name | ||
) | ||
|
||
|
||
@bp.route("/csv/<string:kind>") | ||
|
@@ -199,3 +233,91 @@ def delete_class() -> Response: | |
|
||
switch_class(None) | ||
return redirect(url_for("profile.main")) | ||
|
||
@bp.route("/class/config/save", methods=["POST"]) | ||
def save_class_config() -> Response: | ||
db = get_db() | ||
cur_class = get_auth_class() # Use get_auth_class() instead of get_auth() | ||
class_id = cur_class.class_id | ||
|
||
query_limit_enabled = 'query_limit_enabled' in request.form | ||
max_queries = int(request.form.get('max_queries', 50)) # Default to 50 if not provided | ||
|
||
db.execute(""" | ||
UPDATE classes | ||
SET query_limit_enabled = ?, max_queries = ? | ||
WHERE id = ? | ||
""", [query_limit_enabled, max_queries, class_id]) | ||
db.commit() | ||
|
||
flash("Class configuration updated.", "success") | ||
return redirect(url_for("class_config.config_form")) | ||
|
||
@bp.route("/class/reset_queries", methods=["POST"]) | ||
def reset_queries() -> Response: | ||
db = get_db() | ||
cur_class = get_auth_class() # Use get_auth_class() instead of get_auth() | ||
class_id = cur_class.class_id | ||
|
||
db.execute(""" | ||
UPDATE users | ||
SET queries_used = 0 | ||
WHERE id IN ( | ||
SELECT user_id | ||
FROM roles | ||
WHERE class_id = ? AND role = 'student' | ||
) | ||
""", [class_id]) | ||
db.commit() | ||
|
||
flash("Query counts reset for all students.", "success") | ||
return redirect(url_for("instructor.main")) # Redirect to instructor main page | ||
|
||
@bp.route("/reset_student_queries/<int:user_id>", methods=["POST"]) | ||
def reset_student_queries(user_id: int) -> Response: | ||
db = get_db() | ||
cur_class = get_auth_class() | ||
class_id = cur_class.class_id | ||
|
||
# Verify user belongs to class and is a student | ||
student = db.execute(""" | ||
SELECT users.id | ||
FROM users | ||
JOIN roles ON roles.user_id = users.id | ||
WHERE users.id = ? AND roles.class_id = ? AND roles.role = 'student' | ||
""", [user_id, class_id]).fetchone() | ||
|
||
if not student: | ||
flash("Invalid student ID.", "error") | ||
return redirect(url_for("instructor.main")) | ||
|
||
# Reset queries for student | ||
db.execute( | ||
"UPDATE users SET queries_used = 0 WHERE id = ?", | ||
[user_id] | ||
) | ||
db.commit() | ||
|
||
flash("Query count reset for student.", "success") | ||
return redirect(url_for("instructor.main")) | ||
|
||
@bp.route("/reset_all_queries", methods=["POST"]) | ||
def reset_all_queries() -> Response: | ||
db = get_db() | ||
cur_class = get_auth_class() | ||
class_id = cur_class.class_id | ||
|
||
# Reset queries for all students in class | ||
db.execute(""" | ||
UPDATE users | ||
SET queries_used = 0 | ||
WHERE id IN ( | ||
SELECT user_id | ||
FROM roles | ||
WHERE class_id = ? AND role = 'student' | ||
) | ||
""", [class_id]) | ||
db.commit() | ||
|
||
flash("Query counts reset for all students.", "success") | ||
return redirect(url_for("instructor.main")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this needs to be a separate column, or if it would work (and be a bit simpler) to just have a query_limit column that's either an integer (when there is a limit active) or NULL when there is no limit in a class. I'm leaning toward the simpler design, unless there's a reason I'm not seeing to have two columns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the simpler design would definitely work. We would just need to modify a few SQL statements that are looking for this column to check if the query_limit is either NULL or an integer. And of course, set the value accordingly when the instructor either enables or disables this feature.