feat: enhance XBlock structure and update existing APIs by Faraz32123 · Pull Request #4 · edx/gamesxblock

Faraz32123 · 2025-11-19T09:41:10Z

In this commit,

Add save API functionality and made sure previous code is there. We'll use whats there or write new code accordingly, Will do cleansing in the end once its complete.

Previous related PR #3 where we removed old code(We can keep it for now and clean it in the end as some of that code is working).

Screen.Recording.2025-11-19.at.2.37.27.PM.mov

In this commit, - Add save API functionality and made sure previous code is there. We'll use whats there or write new code accordingly, Will do cleansing in the end once its complete. Co-authored-by: [Muhammad Faraz Maqsood] <fmaqsood@2u.com> Co-authored-by: [Vivek Jagdishbhai Ambaliya] <vjagdishbhaiambaliya-apphelix@2u.com>

Copilot

Pull Request Overview

This PR aims to restore previously removed code while adding new save API functionality for the games XBlock. The changes include updating package metadata, adding image upload and settings persistence handlers, and renaming fields for consistency.

Key Changes

Added new API handlers (save_settings, get_settings, upload_image) for managing game configuration
Renamed type field to game_type and list to cards throughout the codebase for consistency
Updated package metadata in setup.py with proper versioning, dependencies, and entry points

Reviewed Changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
setup.py	Updated package metadata, version to 1.0.0, added proper dependencies (XBlock, web-fragments, Django), and fixed entry point path
games/games.py	Added new API handlers for saving settings and uploading images, renamed fields from `type` to `game_type`, introduced duplicate field and import issues
games/static/js/src/games.js	Updated references from `type` to `game_type`, commented out timer update code, added new card loading/display functions
games/static/html/games.html	Changed template variable from `self.title` to `self.game_type`
.gitignore	Added patterns to ignore build artifacts (*.egg-info/, dist/, build/)
games_xblock.egg-info/*	Removed generated egg-info files (should be gitignored)
.DS_Store	Added macOS system file (should not be committed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…tructure

feat: implement code structure with constants and handlers for games

Copilot

Pull Request Overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 14 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-21T06:42:15Z

    )

+    # Alias for cards - handlers.py uses 'list' field name
+    list = cards


Field aliasing via assignment creates a reference to the default value rather than the field itself. This means xblock.list will always reference the empty list [] and won't reflect changes to xblock.cards. Use a property instead or update all handler references to use cards directly.

Suggested change

list = cards

@property

def list(self):

return self.cards

Copilot · 2025-11-21T06:42:15Z

+        }
+        """
+        try:
+            new_game_type = data.get("game_type", GAME_TYPE.FLASHCARDS)


Missing validation for game_type input. The handler should validate that the provided game_type is one of the valid values defined in GAME_TYPE.VALID to prevent invalid game types from being saved.

Suggested change

new_game_type = data.get("game_type", GAME_TYPE.FLASHCARDS)

new_game_type = data.get("game_type", GAME_TYPE.FLASHCARDS)

if new_game_type not in GAME_TYPE.VALID:

return {

"success": False,

"error": f"Invalid game_type '{new_game_type}'. Must be one of: {', '.join(GAME_TYPE.VALID)}"

}

Copilot · 2025-11-21T06:42:15Z

+        if (!data.cards || data.cards.length === 0) {
+            html = '<p class="no-cards">No cards available yet. Add cards in the Studio editor.</p>';
+        } else {
+            html += '<h3>Game Type: ' + data.game_type + '</h3>';


Potential XSS vulnerability. User-controlled data data.game_type is directly concatenated into HTML without escaping. Use proper HTML escaping or DOM manipulation methods to prevent XSS attacks.

Copilot · 2025-11-21T06:42:16Z

+                html += '<div class="card-number">Card ' + (index + 1) + '</div>';
+
+                if (card.term_image) {
+                    html += '<div class="card-image"><img src="' + card.term_image + '" alt="Term image"></div>';


Potential XSS vulnerability. The card.term_image URL is directly concatenated into HTML without validation or escaping. Malicious URLs could contain JavaScript. Similar issue exists for card.definition_image on line 587.

Copilot · 2025-11-21T06:42:16Z

+        var html = '';
+
+        if (!data.cards || data.cards.length === 0) {
+            html = '<p class="no-cards">No cards available yet. Add cards in the Studio editor.</p>';
+        } else {
+            html += '<h3>Game Type: ' + data.game_type + '</h3>';
+            html += '<div class="cards-list">';
+
+            data.cards.forEach(function(card, index) {
+                html += '<div class="card-item">';
+                html += '<div class="card-number">Card ' + (index + 1) + '</div>';
+
+                if (card.term_image) {
+                    html += '<div class="card-image"><img src="' + card.term_image + '" alt="Term image"></div>';
+                }
+                html += '<div class="card-term"><strong>Term:</strong> ' + card.term + '</div>';
+
+                if (card.definition_image) {
+                    html += '<div class="card-image"><img src="' + card.definition_image + '" alt="Definition image"></div>';
+                }
+                html += '<div class="card-definition"><strong>Definition:</strong> ' + card.definition + '</div>';
+
+                html += '</div>';
+            });
+
+            html += '</div>';
+        }
+
+        container.html(html);


Potential XSS vulnerability. The card.term and card.definition (line 589) text content is directly concatenated into HTML without escaping. Use proper HTML escaping or text-safe DOM methods like textContent.

Suggested change

var html = '';

if (!data.cards || data.cards.length === 0) {

html = '<p class="no-cards">No cards available yet. Add cards in the Studio editor.</p>';

} else {

html += '<h3>Game Type: ' + data.game_type + '</h3>';

html += '<div class="cards-list">';

data.cards.forEach(function(card, index) {

html += '<div class="card-item">';

html += '<div class="card-number">Card ' + (index + 1) + '</div>';

if (card.term_image) {

html += '<div class="card-image"><img src="' + card.term_image + '" alt="Term image"></div>';

}

html += '<div class="card-term"><strong>Term:</strong> ' + card.term + '</div>';

if (card.definition_image) {

html += '<div class="card-image"><img src="' + card.definition_image + '" alt="Definition image"></div>';

}

html += '<div class="card-definition"><strong>Definition:</strong> ' + card.definition + '</div>';

html += '</div>';

});

html += '</div>';

}

container.html(html);

container.empty();

if (!data.cards || data.cards.length === 0) {

container.append($('<p class="no-cards">No cards available yet. Add cards in the Studio editor.</p>'));

} else {

container.append($('<h3>').text('Game Type: ' + data.game_type));

var cardsList = $('<div class="cards-list"></div>');

data.cards.forEach(function(card, index) {

var cardItem = $('<div class="card-item"></div>');

cardItem.append($('<div class="card-number"></div>').text('Card ' + (index + 1)));

if (card.term_image) {

cardItem.append($('<div class="card-image"></div>').append(

$('<img>').attr('src', card.term_image).attr('alt', 'Term image')

));

}

var cardTerm = $('<div class="card-term"></div>');

cardTerm.append($('<strong>').text('Term:'));

cardTerm.append(' ');

cardTerm.append(document.createTextNode(card.term));

cardItem.append(cardTerm);

if (card.definition_image) {

cardItem.append($('<div class="card-image"></div>').append(

$('<img>').attr('src', card.definition_image).attr('alt', 'Definition image')

));

}

var cardDef = $('<div class="card-definition"></div>');

cardDef.append($('<strong>').text('Definition:'));

cardDef.append(' ');

cardDef.append(document.createTextNode(card.definition));

cardItem.append(cardDef);

cardsList.append(cardItem);

});

container.append(cardsList);

}

Copilot · 2025-11-21T06:42:17Z

+                validated_cards.append(
+                    {
+                        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, ""),


Empty strings are used as defaults after validation confirms term and definition exist. This means if the validation passes but the values are empty strings, blank cards will be saved. Consider validating that term and definition are non-empty strings, not just present in the dictionary.

Suggested change

validated_cards.append(

{

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, ""),

# Ensure term and definition are non-empty strings

term = card.get(CARD_FIELD.TERM, "")

definition = card.get(CARD_FIELD.DEFINITION, "")

if not isinstance(term, str) or not term.strip():

return {

"success": False,

"error": "Each card's term must be a non-empty string",

}

if not isinstance(definition, str) or not definition.strip():

return {

"success": False,

"error": "Each card's definition must be a non-empty string",

}

validated_cards.append(

{

CARD_FIELD.TERM: term,

CARD_FIELD.TERM_IMAGE: card.get(CARD_FIELD.TERM_IMAGE, ""),

CARD_FIELD.DEFINITION: definition,

Copilot · 2025-11-21T06:42:17Z

    list_length = Integer(
-        default=len(list.default),
+        default=len(cards.default),
        scope=Scope.content,
-        help="A field for the length of the list for convenience."
-    )
-
-    #Flashcard game fields------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-    list_index = Integer(
-        default=0,
-        scope=Scope.settings,
-        help="Determines which flashcard from the list is currently visible."
+        help="A field for the length of the list for convenience.",
    )


Storing list_length as a separate field creates a potential inconsistency where list_length could become out of sync with the actual length of cards. Consider using a property or computing this dynamically from len(self.cards) when needed.

Copilot · 2025-11-21T06:42:17Z

    });

-    return {};
+    // return {};


[nitpick] Commented-out return statement suggests incomplete refactoring. The function should either return an object as before or the comment should be removed if the return is no longer needed.

Suggested change

// return {};

Copilot · 2025-11-21T06:42:17Z

+        except Exception as e:
+            return Response(json_body={"success": False, "error": str(e)}, status=400)


Exposing raw exception messages to clients can leak sensitive implementation details. Consider logging the full exception and returning a generic error message to the client, or at least sanitize the error message.

Copilot · 2025-11-21T06:42:18Z

+        except Exception as e:
+            return {"success": False, "error": str(e)}


Exposing raw exception messages can leak sensitive implementation details. Consider logging the exception and returning a sanitized error message. Similar issue to upload_image handler.

Copilot AI review requested due to automatic review settings November 19, 2025 09:41

Faraz32123 assigned Faraz32123 and viv-helix Nov 19, 2025

Copilot started reviewing on behalf of Faraz32123 November 19, 2025 09:41 View session

Faraz32123 changed the title ~~fix: recover previous code as well~~ Feat: Add save API functionality Nov 19, 2025

Copilot finished reviewing on behalf of Faraz32123 November 19, 2025 09:44

Copilot AI reviewed Nov 19, 2025

View reviewed changes

Faraz32123 mentioned this pull request Nov 20, 2025

feat: refactor code and created new API for Save functionality #3

Closed

viv-helix and others added 6 commits November 20, 2025 14:30

refactor: improve gamexblock API code

6680802

feat: add constants and handler methods for Games XBlock

4c60ced

refactor: organize game handlers and remove unused imports

b8a710c

refactor: reorganize handler methods into separate files for better s…

ae55a4d

…tructure

fix: remove .DS_Store from repository and update .gitignore

243bfb6

Merge pull request #5 from edx/feat/organize_code_structure

b98e68a

feat: implement code structure with constants and handlers for games

Copilot AI review requested due to automatic review settings November 21, 2025 06:37

Copilot started reviewing on behalf of papphelix November 21, 2025 06:37 View session

papphelix approved these changes Nov 21, 2025

View reviewed changes

Copilot finished reviewing on behalf of papphelix November 21, 2025 06:40

Copilot AI reviewed Nov 21, 2025

View reviewed changes

viv-helix changed the title ~~Feat: Add save API functionality~~ feat: enhance XBlock structure and update existing APIs Nov 21, 2025

viv-helix approved these changes Nov 21, 2025

View reviewed changes

viv-helix merged commit 68dae13 into main Nov 21, 2025
7 checks passed

papphelix deleted the feat/refactor_and_implement_save_API_functionality branch November 21, 2025 15:24

-    list = cards
+    @property
+    def list(self):
+        return self.cards

		except Exception as e:
		return Response(json_body={"success": False, "error": str(e)}, status=400)

		except Exception as e:
		return {"success": False, "error": str(e)}

Conversation

Faraz32123 commented Nov 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants