From 053518d02050392c8cc68deca1d127cdfd4473bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 07:52:39 +0000 Subject: [PATCH 1/3] Fix SQL syntax error in clause_discussions_commented_by_user for users with no comments Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com> --- .../gameplan/doctype/gp_discussion/api.py | 14 +++---- .../gp_discussion/test_gp_discussion.py | 41 ++++++++++++++++++- test_fix.py | 38 +++++++++++++++++ 3 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 test_fix.py diff --git a/gameplan/gameplan/doctype/gp_discussion/api.py b/gameplan/gameplan/doctype/gp_discussion/api.py index bfa43ebe6..1b6b3bc7b 100644 --- a/gameplan/gameplan/doctype/gp_discussion/api.py +++ b/gameplan/gameplan/doctype/gp_discussion/api.py @@ -189,15 +189,11 @@ def include_last_post_content(discussions): def clause_discussions_commented_by_user(user): Discussion = frappe.qb.DocType("GP Discussion") - commented_in = list( - set( - frappe.db.get_all( - "GP Comment", - fields=["reference_name"], - filters={"reference_doctype": "GP Discussion", "owner": user}, - pluck="reference_name", - ) - ) + Comment = frappe.qb.DocType("GP Comment") + commented_in = ( + frappe.qb.from_(Comment) + .select(Comment.reference_name) + .where((Comment.reference_doctype == "GP Discussion") & (Comment.owner == user)) ) return Discussion.name.isin(commented_in) diff --git a/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py b/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py index 0e3f99aaf..02c6307d6 100644 --- a/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py +++ b/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py @@ -1,9 +1,46 @@ # Copyright (c) 2023, Frappe Technologies Pvt Ltd and Contributors # See license.txt -# import frappe +import frappe from frappe.tests.utils import FrappeTestCase +from gameplan.gameplan.doctype.gp_discussion.api import clause_discussions_commented_by_user + class TestGPDiscussion(FrappeTestCase): - pass + def test_clause_discussions_commented_by_user_with_no_comments(self): + """Test that clause_discussions_commented_by_user handles users with no comments gracefully""" + # Create a test user who has no comments + test_user = "test_no_comments@example.com" + if not frappe.db.exists("User", test_user): + frappe.get_doc( + { + "doctype": "User", + "email": test_user, + "first_name": "Test", + "last_name": "NoComments", + "send_welcome_email": 0, + } + ).insert(ignore_permissions=True) + + # Ensure the user has no comments + frappe.db.delete("GP Comment", {"owner": test_user}) + frappe.db.commit() + + # This should not raise an SQL error (MariaDB error 1064) + clause = clause_discussions_commented_by_user(test_user) + + # The clause should be valid and can be used in a query + Discussion = frappe.qb.DocType("GP Discussion") + query = frappe.qb.from_(Discussion).select(Discussion.name).where(clause).limit(1) + + # This should execute without error + try: + result = query.run() + # If we get here, the query executed successfully + self.assertIsInstance(result, list) + except Exception as e: + self.fail(f"Query execution failed with error: {e}") + + # Cleanup + frappe.db.rollback() diff --git a/test_fix.py b/test_fix.py new file mode 100644 index 000000000..835ca9ed5 --- /dev/null +++ b/test_fix.py @@ -0,0 +1,38 @@ +#!/usr/bin/env python3 +""" +Test script to verify the fix for clause_discussions_commented_by_user +""" +import sys +import os + +# Add the gameplan directory to the path +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +def test_query_builder_subquery(): + """Test that the query builder generates valid SQL for empty subqueries""" + print("Testing query builder subquery pattern...") + + # Simulate the frappe.qb pattern + # This is a simplified test to ensure the logic is correct + + # Old approach (problematic): + # commented_in = [] # Empty list for user with no comments + # SQL: WHERE name IN () <-- INVALID SQL + + # New approach (fixed): + # commented_in = subquery # Query builder subquery + # SQL: WHERE name IN (SELECT reference_name FROM ...) <-- VALID SQL even if subquery returns no rows + + print("✓ Old approach would create: WHERE name IN ()") + print(" This is invalid SQL and causes MariaDB error 1064") + print() + print("✓ New approach creates: WHERE name IN (SELECT reference_name FROM ...)") + print(" This is valid SQL even when the subquery returns no rows") + print() + print("The fix replaces frappe.db.get_all() (which returns a list)") + print("with frappe.qb subquery (which stays as a query object)") + print() + print("SUCCESS: The fix should resolve the SQL syntax error!") + +if __name__ == "__main__": + test_query_builder_subquery() From ca455cac1cfe910acdb8c9002bb9046d3f01bc75 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 07:52:45 +0000 Subject: [PATCH 2/3] Remove temporary test file Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com> --- test_fix.py | 38 -------------------------------------- 1 file changed, 38 deletions(-) delete mode 100644 test_fix.py diff --git a/test_fix.py b/test_fix.py deleted file mode 100644 index 835ca9ed5..000000000 --- a/test_fix.py +++ /dev/null @@ -1,38 +0,0 @@ -#!/usr/bin/env python3 -""" -Test script to verify the fix for clause_discussions_commented_by_user -""" -import sys -import os - -# Add the gameplan directory to the path -sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) - -def test_query_builder_subquery(): - """Test that the query builder generates valid SQL for empty subqueries""" - print("Testing query builder subquery pattern...") - - # Simulate the frappe.qb pattern - # This is a simplified test to ensure the logic is correct - - # Old approach (problematic): - # commented_in = [] # Empty list for user with no comments - # SQL: WHERE name IN () <-- INVALID SQL - - # New approach (fixed): - # commented_in = subquery # Query builder subquery - # SQL: WHERE name IN (SELECT reference_name FROM ...) <-- VALID SQL even if subquery returns no rows - - print("✓ Old approach would create: WHERE name IN ()") - print(" This is invalid SQL and causes MariaDB error 1064") - print() - print("✓ New approach creates: WHERE name IN (SELECT reference_name FROM ...)") - print(" This is valid SQL even when the subquery returns no rows") - print() - print("The fix replaces frappe.db.get_all() (which returns a list)") - print("with frappe.qb subquery (which stays as a query object)") - print() - print("SUCCESS: The fix should resolve the SQL syntax error!") - -if __name__ == "__main__": - test_query_builder_subquery() From cf3648ab9a76ef799d1cc2e817132e126bfd4c4b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 1 Feb 2026 07:53:36 +0000 Subject: [PATCH 3/3] Address code review feedback: simplify test case Co-authored-by: netchampfaris <9355208+netchampfaris@users.noreply.github.com> --- .../gameplan/doctype/gp_discussion/test_gp_discussion.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py b/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py index 02c6307d6..7113786a2 100644 --- a/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py +++ b/gameplan/gameplan/doctype/gp_discussion/test_gp_discussion.py @@ -25,7 +25,6 @@ def test_clause_discussions_commented_by_user_with_no_comments(self): # Ensure the user has no comments frappe.db.delete("GP Comment", {"owner": test_user}) - frappe.db.commit() # This should not raise an SQL error (MariaDB error 1064) clause = clause_discussions_commented_by_user(test_user) @@ -35,12 +34,8 @@ def test_clause_discussions_commented_by_user_with_no_comments(self): query = frappe.qb.from_(Discussion).select(Discussion.name).where(clause).limit(1) # This should execute without error - try: - result = query.run() - # If we get here, the query executed successfully - self.assertIsInstance(result, list) - except Exception as e: - self.fail(f"Query execution failed with error: {e}") + result = query.run() + self.assertIsInstance(result, list) # Cleanup frappe.db.rollback()