Adding delet round functionality

[Ashish-Ramrakhiani]

User description

The code review will be triggered by codium ai

---

PR Type

Enhancement

---

Description

• Added round deletion functionality with confirmation modal

• Implemented deleteRound and confirmDelete JavaScript functions

• Updated HTML to include confirmation modals for round deletion

• Enhanced user feedback and safety for destructive actions

---

Changes walkthrough 📝

Relevant files

Enhancement

roundsMode.js Implement round deletion logic and confirmation handling  scripts/roundsMode.js Added deleteRound function to remove rounds from data and log deletion Added confirmDelete function to show modal and handle user confirmation Updated table and local storage upon deletion Introduced global variable and placeholder updateRoundsList function +70/-0    index.html Add and enhance confirmation modals for round deletion      index.html Added two modal dialogs for confirming round deletion Enhanced modal with warning for irreversible action Added new buttons for canceling or confirming deletion Linked modal buttons to deletion logic +83/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Duplicate Modal ID The confirmDelete function has a hardcoded roundId = 1 that overrides the parameter, making it impossible to delete any round except the one with ID 1. var roundId = 1; Duplicate Modal There are two modal dialogs with the same ID 'confirmDeleteRoundModal', which will cause conflicts when trying to show the modal. <div class="modal" tabindex="-1" id="confirmDeleteRoundModal"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <h5 class="modal-title">Delete Round?</h5> <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close" ></button> </div> <div class="modal-body"> <p>Do you really want to delete that round?</p> </div> <div class="modal-footer"> <button type="button" class="btn btn-secondary" data-bs-dismiss="modal" > No, Cancel </button> <button type="button" id="confirmDeleteBtn" class="btn btn-primary" onClick="" > Yes, Delete Round </button> </div> </div> </div> </div> <div class="modal" tabindex="-1" id="confirmDeleteRoundModal"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <h5 class="modal-title">Delete Round?</h5> <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close" onclick="javascript:void(0);" ></button> </div> <div class="modal-body"> <p>Do you really want to delete that round?</p> <p id='deleteWarning' style="color: red;"> This action cannot be undone! </p> </div> <div class="modal-footer"> <button type="button" class="btn btn-secondary" data-bs-dismiss="modal" > No, Cancel </button> <button type="button" id="confirmDeleteBtn" class="btn btn-primary" onClick="" > Yes, Delete Round </button> <button type="button" id="confirmDeleteBtn2" class="btn btn-danger" onClick="deleteRound(roundId)" > Yes, Permanently Delete </button> </div> </div> </div> </div> Global Variable An undeclared global variable is introduced without proper initialization or purpose, which could lead to unexpected behavior. undeclaredGlobalVariable = "This will be a global variable";

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix parameter override The function parameter roundId is being immediately overwritten with a hardcoded value of 1, causing all deletions to target round #1 regardless of which round the user selected. Remove the redeclaration to use the passed parameter. scripts/roundsMode.js [244-245] function confirmDelete(roundId) { - var roundId = 1; - let modal = new bootstrap.Modal( document.getElementById("confirmDeleteRoundModal") ); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 10 __ Why: This suggestion identifies a critical logic error where the roundId parameter is being overwritten with a hardcoded value of 1, which would cause the wrong round to be deleted regardless of user selection. This is a severe functional bug.	High
	Fix event listener leak The deleteRound function adds a new event listener every time it's called, creating multiple redundant listeners. This causes the deletion message to be logged multiple times for every click anywhere in the document. Remove the event listener or replace it with a direct console log. scripts/roundsMode.js [227-235] function deleteRound(roundId) { GlobalUserData.rounds = GlobalUserData.rounds.filter(function (round) { return round.roundNum !== roundId; }); - document.addEventListener('click', function() { - console.log("Round deleted: " + roundId); - }); + console.log("Round deleted: " + roundId); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a serious issue where event listeners are being added repeatedly without removal, causing memory leaks and multiple log messages for each click. This is a critical bug that would affect application performance.	High
	Remove duplicate modal There are two modal dialogs with the same ID confirmDeleteRoundModal, which will cause conflicts when trying to show the modal. Remove the duplicate modal or give it a different ID. index.html [1560-1596] <div class="modal" tabindex="-1" id="confirmDeleteRoundModal"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <h5 class="modal-title">Delete Round?</h5> <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close" ></button> </div> ... </div> </div> </div> -<div class="modal" tabindex="-1" id="confirmDeleteRoundModal"> - [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies duplicate modal elements with the same ID, which would cause DOM conflicts and JavaScript errors. This is a significant issue that would prevent the deletion confirmation functionality from working properly.	High
Possible issue	Fix array bounds check The loop condition uses <= which will cause an out-of-bounds error when accessing the last index. Change to < to prevent accessing an index that doesn't exist. scripts/roundsMode.js [256-262] -for (var i = 0; i <= GlobalRoundsTable.rows.length; i++) { +for (var i = 0; i < GlobalRoundsTable.rows.length; i++) { let row = GlobalRoundsTable.rows[i]; if (row.id === "r-" + roundId) { GlobalRoundsTable.deleteRow(i); break; } } Apply / Chat Suggestion importance[1-10]: 10 __ Why: This is a critical bug fix. Using <= in the loop condition would cause an out-of-bounds error when accessing GlobalRoundsTable.rows[length], which doesn't exist. This could lead to runtime errors and application crashes.	High
	Fix undefined variable reference The onClick handler references roundId directly, which may not be in scope in the HTML context. Pass the roundId through a function that has access to the correct variable. index.html [1631-1638] <button type="button" id="confirmDeleteBtn2" class="btn btn-danger" - onClick="deleteRound(roundId)" + onClick="confirmDelete(this.getAttribute('data-round-id'))" + data-round-id="" > Yes, Permanently Delete </button> Apply / Chat Suggestion importance[1-10]: 9 __ Why: The current code references roundId which likely isn't defined in the HTML context, causing a runtime error when clicking the button. The suggested data attribute approach properly passes the round ID to the delete function.	High
General	Declare global variable properly Avoid using undeclared global variables as they can lead to unexpected behavior and conflicts. Either declare the variable with let, const, or var, or remove it if not needed. scripts/roundsMode.js [284] -undeclaredGlobalVariable = "This will be a global variable"; +const undeclaredGlobalVariable = "This will be a global variable"; Apply / Chat Suggestion importance[1-10]: 8 __ Why: Using undeclared global variables is a significant issue that can lead to scope problems, naming conflicts, and hard-to-debug errors. Properly declaring the variable with const improves code quality and prevents potential runtime issues.	Medium
Possible issue	Inconsistent state management The second delete button bypasses the confirmDelete function and directly calls deleteRound, which doesn't update the UI or localStorage. This creates inconsistent state. index.html [1631-1638] <button type="button" id="confirmDeleteBtn2" class="btn btn-danger" - onClick="deleteRound(roundId)" + onClick="confirmDelete(roundId)" > Yes, Permanently Delete </button> Apply / Chat Suggestion importance[1-10]: 9 __ Why: The second delete button directly calls deleteRound() which bypasses important UI updates and localStorage persistence that are handled in the confirmDelete() function. This could lead to data inconsistency where the UI shows one state but the actual data is in another state.	High
General	Return value missing The deleteRound function doesn't return any value despite its JSDoc indicating it should return a boolean. Add a return statement to match the documented behavior. scripts/roundsMode.js [227-235] function deleteRound(roundId) { GlobalUserData.rounds = GlobalUserData.rounds.filter(function (round) { return round.roundNum !== roundId; }); document.addEventListener('click', function() { console.log("Round deleted: " + roundId); }); + + return true; } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The function deleteRound is documented to return a boolean value (true/false) but doesn't actually return anything. Adding the return statement makes the function match its documented behavior, which is important for code correctness and reliability.	Medium
	Unused function defined The updateRoundsList function is defined but never used in the code. Either implement its functionality or remove it to avoid dead code. scripts/roundsMode.js [286-288] +// Either implement this function or remove it if not needed function updateRoundsList(userId, timestamp, data, extraParam) { console.log("Updating rounds for user: " + userId); + // Implementation needed here } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The updateRoundsList function is defined but appears to be unused in the code. While not a critical issue, removing or implementing unused functions improves code maintainability and reduces confusion for developers.	Low
Possible issue	Persist data after deletion The deleteRound function doesn't update the UI or save changes to localStorage, which means deleted rounds will reappear when the page refreshes. Add localStorage update to persist the changes. scripts/roundsMode.js [227-235] function deleteRound(roundId) { GlobalUserData.rounds = GlobalUserData.rounds.filter(function (round) { return round.roundNum !== roundId; }); document.addEventListener('click', function() { console.log("Round deleted: " + roundId); }); + + localStorage.setItem( + GlobalUserData.accountInfo.email, + JSON.stringify(GlobalUserData) + ); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that deleteRound() doesn't persist changes to localStorage, which would cause deleted rounds to reappear on page refresh. However, the PR already handles localStorage updates in the confirmDelete() function (lines 266-269) after calling deleteRound(), so this is more of a design choice than a critical issue.	Medium
Possible issue	Fix empty onClick attribute The onClick attribute is empty, which means the button doesn't trigger any action when clicked. This contradicts the button's purpose of confirming deletion. index.html [1623-1630] <button type="button" id="confirmDeleteBtn" class="btn btn-primary" - onClick="" > Yes, Delete Round </button> Apply / Chat Suggestion importance[1-10]: 7 __ Why: The empty onClick="" attribute is problematic as it suggests the button should perform an action but doesn't specify what action. This is inconsistent with the button's purpose of confirming deletion and could lead to user confusion when nothing happens upon clicking.	Medium
General	Remove unused function parameters The updateRoundsList function is defined with parameters but doesn't use most of them. Either implement the function properly to use all parameters or remove unused parameters to avoid confusion. scripts/roundsMode.js [286-288] -function updateRoundsList(userId, timestamp, data, extraParam) { +function updateRoundsList(userId) { console.log("Updating rounds for user: " + userId); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The function updateRoundsList declares four parameters but only uses the first one (userId). Removing unused parameters improves code clarity and maintainability by eliminating potential confusion about their purpose.	Low
	Remove redundant onclick handler The onclick="javascript:void(0);" is redundant and unnecessary. The data-bs-dismiss="modal" attribute already handles the close functionality with Bootstrap. index.html [1601-1607] <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close" - onclick="javascript:void(0);" ></button> Apply / Chat Suggestion importance[1-10]: 3 __ Why: The onclick="javascript:void(0);" is indeed redundant since the data-bs-dismiss="modal" attribute already handles the close functionality in Bootstrap. Removing it makes the code cleaner without affecting functionality.	Low
More

[Ashish-Ramrakhiani]

/codium review