Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion games/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@

class GAME_TYPE:
"""Game type constants."""

FLASHCARDS = "flashcards"
MATCHING = "matching"
VALID = [FLASHCARDS, MATCHING]


class DEFAULT:
"""Default values for XBlock fields."""

TITLE = "Matching"
DISPLAY_NAME = "Games"
GAME_TYPE = GAME_TYPE.MATCHING
Expand All @@ -26,6 +28,7 @@ class DEFAULT:

class CARD_FIELD:
"""Card field names."""

TERM = "term"
TERM_IMAGE = "term_image"
DEFINITION = "definition"
Expand All @@ -35,17 +38,19 @@ class CARD_FIELD:

class CONTAINER_TYPE:
"""Container types for matching game."""

TERM = "term"
DEFINITION = "definition"


class UPLOAD:
"""File upload settings."""

PATH_PREFIX = "games"
DEFAULT_EXTENSION = "jpg"


class CONFIG:
"""Configuration values."""

RANDOM_STRING_LENGTH = 6
MATCHES_PER_PAGE = 5 # Number of matches displayed per page
62 changes: 44 additions & 18 deletions games/games.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from .constants import DEFAULT
from .handlers import CommonHandlers, FlashcardsHandlers, MatchingHandlers


class GamesXBlock(XBlock):
"""
An XBlock for creating games.
Expand All @@ -25,14 +26,18 @@ class GamesXBlock(XBlock):
help=_("The title of the block to be displayed in the xblock."),
)
display_name = String(
default=DEFAULT.DISPLAY_NAME, scope=Scope.settings, help=_("Display name for this XBlock")
default=DEFAULT.DISPLAY_NAME,
scope=Scope.settings,
help="Display name for this XBlock",
)

# Change default to 'matching' for matching game and 'flashcards' for flashcards game to test
game_type = String(
default=DEFAULT.GAME_TYPE,
scope=Scope.settings,
help=_("The kind of game this xblock is responsible for ('flashcards' or 'matching' for now)."),
help=_(
"The kind of game this xblock is responsible for ('flashcards' or 'matching' for now)."
),
)

cards = List(
Expand Down Expand Up @@ -64,19 +69,25 @@ class GamesXBlock(XBlock):
matching_id_dictionary_index = Dict(
default={},
scope=Scope.user_state,
help=_("A dictionary to encrypt the ids of the terms and definitions for the matching game."),
help=_(
"A dictionary to encrypt the ids of the terms and definitions for the matching game."
),
)

matching_id_dictionary_type = Dict(
default={},
scope=Scope.user_state,
help=_("A dictionary to tie the id to the type of container (term or definition) for the matching game."),
help=_(
"A dictionary to tie the id to the type of container (term or definition) for the matching game."
),
)

matching_id_dictionary = Dict(
default={},
scope=Scope.user_state,
help=_("A dictionary to encrypt the ids of the terms and definitions for the matching game."),
help=_(
"A dictionary to encrypt the ids of the terms and definitions for the matching game."
),
)

matching_id_list = List(
Expand Down Expand Up @@ -106,7 +117,9 @@ class GamesXBlock(XBlock):
match_count = Integer(
default=DEFAULT.MATCH_COUNT,
scope=Scope.user_state,
help=_("Tracks how many matches have been successfully made. Used to determine when to switch pages."),
help=_(
"Tracks how many matches have been successfully made. Used to determine when to switch pages."
),
)

matches_remaining = Integer(
Expand All @@ -116,7 +129,9 @@ class GamesXBlock(XBlock):
selected_containers = Dict(
default={},
scope=Scope.user_state,
help=_("A dictionary to keep track of selected containers for the matching game."),
help=_(
"A dictionary to keep track of selected containers for the matching game."
),
)

is_shuffled = Boolean(
Expand Down Expand Up @@ -153,7 +168,7 @@ def student_view(self, context=None):

# Common handlers------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@XBlock.json_handler
def expand_game(self, data, suffix=''):
def expand_game(self, data, suffix=""):
"""A handler to expand the game from its title block."""
return CommonHandlers.expand_game(self, data, suffix)

Expand All @@ -164,58 +179,69 @@ def get_settings(self, data, suffix=""):

@XBlock.handler
def upload_image(self, request, suffix=""):
"""Upload an image file and return the URL."""
"""
Upload an image file to configured storage (S3 if set) and return URL.
"""
return CommonHandlers.upload_image(self, request, suffix)

@XBlock.json_handler
def delete_image_handler(self, data, suffix=""):
"""
Delete an image by storage key.
Expected: { "key": "gamesxblock/<block_id>/<hash>.ext" }
"""
# TODO: Delete API is not integrated yet, will handle this one after API is integrated if any change needed.
return CommonHandlers.delete_image(self, data, suffix)
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

Calls the wrong method name. The handler is defined as delete_image_handler in common.py but called as delete_image here. Change to CommonHandlers.delete_image_handler(self, data, suffix) to match the actual method name.

Suggested change
return CommonHandlers.delete_image(self, data, suffix)
return CommonHandlers.delete_image_handler(self, data, suffix)

Copilot uses AI. Check for mistakes.

@XBlock.json_handler
def save_settings(self, data, suffix=""):
"""Save game type, shuffle setting, and all cards in one API call."""
return CommonHandlers.save_settings(self, data, suffix)

@XBlock.json_handler
def close_game(self, data, suffix=''):
def close_game(self, data, suffix=""):
"""A handler to close the game to its title block."""
return CommonHandlers.close_game(self, data, suffix)

@XBlock.json_handler
def display_help(self, data, suffix=''):
def display_help(self, data, suffix=""):
"""A handler to display a tooltip message above the help icon."""
return CommonHandlers.display_help(self, data, suffix)

# Flashcards handlers------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@XBlock.json_handler
def start_game_flashcards(self, data, suffix=''):
def start_game_flashcards(self, data, suffix=""):
"""A handler to begin the flashcards game."""
return FlashcardsHandlers.start_game_flashcards(self, data, suffix)

@XBlock.json_handler
def flip_flashcard(self, data, suffix=''):
def flip_flashcard(self, data, suffix=""):
"""A handler to flip the flashcard from term to definition and vice versa."""
return FlashcardsHandlers.flip_flashcard(self, data, suffix)

@XBlock.json_handler
def page_turn(self, data, suffix=''):
def page_turn(self, data, suffix=""):
"""A handler to turn the page to a new flashcard (left or right) in the list."""
return FlashcardsHandlers.page_turn(self, data, suffix)

# Matching handlers------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@XBlock.json_handler
def start_game_matching(self, data, suffix=''):
def start_game_matching(self, data, suffix=""):
"""A handler to begin the matching game."""
return MatchingHandlers.start_game_matching(self, data, suffix)

@XBlock.json_handler
def update_timer(self, data, suffix=''):
def update_timer(self, data, suffix=""):
"""Update the timer. This is called every 1000ms by an ajax call."""
return MatchingHandlers.update_timer(self, data, suffix)

@XBlock.json_handler
def select_container(self, data, suffix=''):
def select_container(self, data, suffix=""):
"""A handler for selecting matching game containers and evaluating matches."""
return MatchingHandlers.select_container(self, data, suffix)

@XBlock.json_handler
def end_game_matching(self, data, suffix=''):
def end_game_matching(self, data, suffix=""):
"""End the matching game and compare the user's time to the best_time field."""
return MatchingHandlers.end_game_matching(self, data, suffix)

Expand Down
2 changes: 1 addition & 1 deletion games/handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
from .flashcards import FlashcardsHandlers
from .matching import MatchingHandlers

__all__ = ['CommonHandlers', 'FlashcardsHandlers', 'MatchingHandlers']
__all__ = ["CommonHandlers", "FlashcardsHandlers", "MatchingHandlers"]
83 changes: 61 additions & 22 deletions games/handlers/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

This module contains handlers that work across all game types.
"""

import hashlib

from django.core.files.base import ContentFile
Expand All @@ -16,23 +17,24 @@
GAME_TYPE,
UPLOAD,
)
from games.utils import get_gamesxblock_storage, delete_image


class CommonHandlers:
"""Handlers that work across all game types."""

@staticmethod
def expand_game(xblock, data, suffix=''):
def expand_game(xblock, data, suffix=""):
"""A handler to expand the game from its title block."""
description = _("ERR: self.game_type not defined or invalid")
if xblock.game_type == GAME_TYPE.FLASHCARDS:
description = _("Click each card to reveal the definition")
elif xblock.game_type == GAME_TYPE.MATCHING:
description = _("Match each term with the correct definition")
return {
'title': xblock.title,
'description': description,
'game_type': xblock.game_type
"title": xblock.title,
"description": description,
"game_type": xblock.game_type,
}

@staticmethod
Expand All @@ -46,26 +48,63 @@ def get_settings(xblock, data, suffix=""):

@staticmethod
def upload_image(xblock, request, suffix=""):
"""Upload an image file and return the URL."""
"""
Upload an image file to configured storage (S3 if set) and return URL.
"""
asset_storage = get_gamesxblock_storage()
try:
upload_file = request.params["file"].file
file_name = request.params["file"].filename
file_hash = hashlib.md5(upload_file.read()).hexdigest()
upload_file.seek(0)
_, ext = (
file_name.rsplit(".", 1) if "." in file_name else (file_name, UPLOAD.DEFAULT_EXTENSION)
)
if "." not in file_name:
return Response(
json_body={
"success": False,
"error": "File must have an extension",
},
status=400,
)
_, ext = file_name.rsplit(".", 1)
ext = ext.lower()
allowed_exts = ["jpg", "jpeg", "png", "gif", "webp", "svg"]
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

[nitpick] The allowed file extensions list is hardcoded in the handler. Consider moving this to a constant in constants.py (e.g., UPLOAD.ALLOWED_EXTENSIONS) to make it easier to maintain and modify without changing handler code.

Suggested change
allowed_exts = ["jpg", "jpeg", "png", "gif", "webp", "svg"]
allowed_exts = UPLOAD.ALLOWED_EXTENSIONS

Copilot uses AI. Check for mistakes.
if ext not in allowed_exts:
return Response(
json_body={
"success": False,
"error": f"Unsupported file type '.{ext}'. Allowed: {', '.join(sorted(allowed_exts))}",
},
status=400,
)
blob = upload_file.read()
file_hash = hashlib.md5(blob).hexdigest()
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

MD5 is cryptographically weak and should not be used for security purposes. While this appears to be used for file deduplication rather than security, consider using a stronger hash like SHA256 to avoid potential collision attacks. Use hashlib.sha256(blob).hexdigest() instead.

Suggested change
file_hash = hashlib.md5(blob).hexdigest()
file_hash = hashlib.sha256(blob).hexdigest()

Copilot uses AI. Check for mistakes.
file_path = f"{UPLOAD.PATH_PREFIX}/{xblock.scope_ids.usage_id.block_id}/{file_hash}.{ext}"
saved_path = default_storage.save(
file_path, ContentFile(upload_file.read())
)
file_url = default_storage.url(saved_path)
saved_path = asset_storage.save(file_path, ContentFile(blob))
file_url = asset_storage.url(saved_path)
return Response(
json_body={"success": True, "url": file_url, "filename": file_name}
json_body={
"success": True,
"url": file_url,
"filename": file_name,
"file_path": file_path,
}
)
except Exception as e:
return Response(json_body={"success": False, "error": str(e)}, status=400)

@staticmethod
def delete_image_handler(self, data, suffix=""):
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

The method signature is inconsistent with other static methods in this class. It has self as the first parameter while being marked as @staticmethod. This will cause a runtime error when called. Change self to xblock to match the pattern used by other static methods like upload_image and save_settings.

Copilot uses AI. Check for mistakes.
"""
Delete an image by storage key.
Expected: { "key": "gamesxblock/<block_id>/<hash>.ext" }
"""
key = data.get("key")
if not key:
return {"success": False, "error": "Missing key"}
try:
is_deleted = delete_image(self.asset_storage, key)
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

References self.asset_storage which doesn't exist. The storage should be obtained via get_gamesxblock_storage() as done in the upload_image method. Change to asset_storage = get_gamesxblock_storage() and then use delete_image(asset_storage, key).

Suggested change
is_deleted = delete_image(self.asset_storage, key)
asset_storage = get_gamesxblock_storage()
is_deleted = delete_image(asset_storage, key)

Copilot uses AI. Check for mistakes.
return {"success": is_deleted, "key": key}
except Exception as e:
return {"success": False, "error": str(e)}

@staticmethod
def save_settings(xblock, data, suffix=""):
"""
Expand Down Expand Up @@ -107,7 +146,9 @@ def save_settings(xblock, data, suffix=""):
CARD_FIELD.TERM: card.get(CARD_FIELD.TERM, ""),
CARD_FIELD.TERM_IMAGE: card.get(CARD_FIELD.TERM_IMAGE, ""),
CARD_FIELD.DEFINITION: card.get(CARD_FIELD.DEFINITION, ""),
CARD_FIELD.DEFINITION_IMAGE: card.get(CARD_FIELD.DEFINITION_IMAGE, ""),
CARD_FIELD.DEFINITION_IMAGE: card.get(
CARD_FIELD.DEFINITION_IMAGE, ""
),
CARD_FIELD.ORDER: card.get(CARD_FIELD.ORDER, ""),
}
)
Expand All @@ -129,7 +170,7 @@ def save_settings(xblock, data, suffix=""):
return {"success": False, "error": str(e)}

@staticmethod
def close_game(xblock, data, suffix=''):
def close_game(xblock, data, suffix=""):
"""A handler to close the game to its title block."""
xblock.game_started = False
xblock.time_seconds = 0
Expand All @@ -139,16 +180,14 @@ def close_game(xblock, data, suffix=''):
if xblock.game_type == GAME_TYPE.FLASHCARDS:
xblock.term_is_visible = True
xblock.list_index = 0
return {
'title': xblock.title
}
return {"title": xblock.title}

@staticmethod
def display_help(xblock, data, suffix=''):
def display_help(xblock, data, suffix=""):
"""A handler to display a tooltip message above the help icon."""
message = _("ERR: self.game_type not defined or invalid")
if xblock.game_type == GAME_TYPE.FLASHCARDS:
message = _("Click each card to reveal the definition")
elif xblock.game_type == GAME_TYPE.MATCHING:
message = _("Match each term with the correct definition")
return {'message': message}
return {"message": message}
Loading
Loading