From 5351932194d86457d3992ba6434d6fd0e4d98f2a Mon Sep 17 00:00:00 2001
From: Ryan Kegel <ryan.kegel@gmail.com>
Date: Sun, 1 Feb 2026 23:39:55 -0500
Subject: [PATCH] feat: Enhance task and reward assignment logic to prioritize
 user items over system items with the same name; add corresponding tests

---
 .github/copilot-instructions.md               |   2 +
 .../bug-both-system-and-user-items-shown.md   |  19 +++
 backend/api/child_api.py                      | 148 +++++++++++-------
 backend/tests/test_child_api.py               |  78 +++++++++
 4 files changed, 193 insertions(+), 54 deletions(-)
 create mode 100644 .github/specs/archive/bug-both-system-and-user-items-shown.md

diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
index c46898a..aa8145b 100644
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -15,6 +15,7 @@
 - **Scoped Styles**: All `.vue` files must use `<style scoped>`. Reference global variables for theme consistency.
 - **API Error Handling**: Backend returns JSON with `error` and `code` (see `backend/api/error_codes.py`). Frontend extracts `{ msg, code }` using `parseErrorResponse(res)` from `api.ts`.
 - **JWT Auth**: Tokens are stored in HttpOnly, Secure, SameSite=Strict cookies.
+- **Code Style**: Follow PEP 8 for Python, and standard TypeScript conventions. Use type annotations everywhere in Python. Place python changes after imports. Place all imports at the top of the file.
 
 ## 🚦 Frontend Logic & Event Bus
 
@@ -29,6 +30,7 @@
 ## 🛠️ Developer Workflows
 
 - **Backend**: Run Flask with `python -m flask run --host=0.0.0.0 --port=5000` from the `backend/` directory. Main entry: `backend/main.py`.
+- **Virtual Env**: Python is running from a virtual environment located at `backend/.venv/`.
 - **Frontend**: From `frontend/vue-app/`, run `npm install` then `npm run dev`.
 - **Tests**: Run backend tests with `pytest` in `backend/tests/`. Frontend component tests: `npm run test` in `frontend/vue-app/components/__tests__/`.
 - **Debugging**: Use VS Code launch configs or run Flask/Vue dev servers directly. For SSE, use browser dev tools to inspect event streams.
diff --git a/.github/specs/archive/bug-both-system-and-user-items-shown.md b/.github/specs/archive/bug-both-system-and-user-items-shown.md
new file mode 100644
index 0000000..b2f5a11
--- /dev/null
+++ b/.github/specs/archive/bug-both-system-and-user-items-shown.md
@@ -0,0 +1,19 @@
+# Bug: When a user task or reward exists with the same name as a system user or task, both are shown in the assign list.
+
+## The Problem
+
+- **Actual:** When the user creates a task/reward from a system task/reward (copy on edit), and then goes to assign the task/reward, both the system and user task/reward are shown and can be assigned.
+- **Expected:** When a user task/reward is created from a system (or even if it has the same name) - show the user item instead in the assign views.
+
+## Investigation Notes
+
+- When a copy on edit happens of a 'good' task and it is changed to 'bad', I can see the 'good' task when assigning tasks and the 'bad' penalty when assigning the penalty
+- The backend will have to change to probably check if the names are the same on tasks/rewards and if so, choose to return the user items instead.
+- In the case of two items having the same name AND having different user_ids that are not null, then we should show both items.
+- The task view and reward view correctly hides the system item. However, the Task Assign View and RewardAssignView are still showing both items.
+
+## The "Red" Tests
+
+ - [x] Create a test that performs a copy on edit and then makes sure only that item shows instead of the system item
+ - [x] Create a test that performs has 2 user items with the same name as a system item. Verify that the user items are shown, but not the system item.
+ - [x] Create a test where if a system and identically named user task exist that the user tasks is the only one shown in the task assign view and reward assign view.
diff --git a/backend/api/child_api.py b/backend/api/child_api.py
index e6caf46..34f9dab 100644
--- a/backend/api/child_api.py
+++ b/backend/api/child_api.py
@@ -22,6 +22,7 @@ from models.pending_reward import PendingReward
 from models.reward import Reward
 from models.task import Task
 from api.utils import get_validated_user_id
+from collections import defaultdict
 import logging
 
 child_api = Blueprint('child_api', __name__)
@@ -259,22 +260,30 @@ def list_assignable_tasks(id):
     child = result[0]
     assigned_ids = set(child.get('tasks', []))
 
-    # Collect all task ids from the task database
-    all_task_ids = [t.get('id') for t in task_db.all() if t and t.get('id')]
+    # Get all assignable tasks (not already assigned)
+    all_tasks = [t for t in task_db.all() if t and t.get('id') and t.get('id') not in assigned_ids]
 
-    # Filter out already assigned
-    assignable_ids = [tid for tid in all_task_ids if tid not in assigned_ids]
+    # Group by name
+    from collections import defaultdict
+    name_to_tasks = defaultdict(list)
+    for t in all_tasks:
+        name_to_tasks[t.get('name')].append(t)
 
-    # Fetch full task details and wrap in ChildTask
-    TaskQuery = Query()
-    assignable_tasks = []
-    for tid in assignable_ids:
-        task = task_db.get((TaskQuery.id == tid) & ((TaskQuery.user_id == user_id) | (TaskQuery.user_id == None)))
-        if not task:
-            continue
-        ct = ChildTask(task.get('name'), task.get('is_good'), task.get('points'), task.get('image_id'), task.get('id'))
-        assignable_tasks.append(ct.to_dict())
+    filtered_tasks = []
+    for name, tasks in name_to_tasks.items():
+        user_tasks = [t for t in tasks if t.get('user_id') is not None]
+        if len(user_tasks) == 0:
+            # Only system task exists
+            filtered_tasks.append(tasks[0])
+        elif len(user_tasks) == 1:
+            # Only one user task: show it, not system
+            filtered_tasks.append(user_tasks[0])
+        else:
+            # Multiple user tasks: show all user tasks, not system
+            filtered_tasks.extend(user_tasks)
 
+    # Wrap in ChildTask and return
+    assignable_tasks = [ChildTask(t.get('name'), t.get('is_good'), t.get('points'), t.get('image_id'), t.get('id')).to_dict() for t in filtered_tasks]
     return jsonify({'tasks': assignable_tasks, 'count': len(assignable_tasks)}), 200
 
 
@@ -294,30 +303,41 @@ def list_all_tasks(id):
 
     child = result[0]
     assigned_ids = set(child.get('tasks', []))
-
-    # Get all tasks from database
+    # Get all tasks from database (not filtering out assigned, since this is 'all')
     ChildTaskQuery = Query()
     all_tasks = task_db.search((ChildTaskQuery.user_id == user_id) | (ChildTaskQuery.user_id == None))
 
-    tasks = []
+    name_to_tasks = defaultdict(list)
+    for t in all_tasks:
+        name_to_tasks[t.get('name')].append(t)
 
-    for task in all_tasks:
+    filtered_tasks = []
+    for name, tasks in name_to_tasks.items():
+        user_tasks = [t for t in tasks if t.get('user_id') is not None]
+        if len(user_tasks) == 0:
+            filtered_tasks.append(tasks[0])
+        elif len(user_tasks) == 1:
+            filtered_tasks.append(user_tasks[0])
+        else:
+            filtered_tasks.extend(user_tasks)
+            
+    result_tasks = []
+    for t in filtered_tasks:
+        if has_type and t.get('is_good') != good:
+            continue
         ct = ChildTask(
-            task.get('name'),
-            task.get('is_good'),
-            task.get('points'),
-            task.get('image_id'),
-            task.get('id')
+            t.get('name'),
+            t.get('is_good'),
+            t.get('points'),
+            t.get('image_id'),
+            t.get('id')
         )
         task_dict = ct.to_dict()
-        if has_type and task.get('is_good') != good:
-            continue
-
-        task_dict.update({'assigned': task.get('id') in assigned_ids})
-        tasks.append(task_dict)
-        tasks.sort(key=lambda t: (not t['assigned'], t['name'].lower()))
-
-    return jsonify({ 'tasks': tasks, 'count': len(tasks), 'list_type': 'task' }), 200
+        task_dict.update({'assigned': t.get('id') in assigned_ids})
+        result_tasks.append(task_dict)            
+        
+    result_tasks.sort(key=lambda t: (not t['assigned'], t['name'].lower()))
+    return jsonify({ 'tasks': result_tasks, 'count': len(result_tasks), 'list_type': 'task' }), 200
 
 
 @child_api.route('/child/<id>/trigger-task', methods=['POST'])
@@ -394,25 +414,37 @@ def list_all_rewards(id):
     # Get all rewards from database
     ChildRewardQuery = Query()
     all_rewards = reward_db.search((ChildRewardQuery.user_id == user_id) | (ChildRewardQuery.user_id == None))
-    rewards = []
 
-    for reward in all_rewards:
+    from collections import defaultdict
+    name_to_rewards = defaultdict(list)
+    for r in all_rewards:
+        name_to_rewards[r.get('name')].append(r)
+
+    filtered_rewards = []
+    for name, rewards in name_to_rewards.items():
+        user_rewards = [r for r in rewards if r.get('user_id') is not None]
+        if len(user_rewards) == 0:
+            filtered_rewards.append(rewards[0])
+        elif len(user_rewards) == 1:
+            filtered_rewards.append(user_rewards[0])
+        else:
+            filtered_rewards.extend(user_rewards)
+
+    result_rewards = []
+    for r in filtered_rewards:
         cr = ChildReward(
-            reward.get('name'),
-            reward.get('cost'),
-            reward.get('image_id'),
-            reward.get('id')
+            r.get('name'),
+            r.get('cost'),
+            r.get('image_id'),
+            r.get('id')
         )
-
         reward_dict = cr.to_dict()
-
-        reward_dict.update({'assigned': reward.get('id') in assigned_ids})
-        rewards.append(reward_dict)
-        rewards.sort(key=lambda t: (not t['assigned'], t['name'].lower()))
-
+        reward_dict.update({'assigned': r.get('id') in assigned_ids})
+        result_rewards.append(reward_dict)
+    result_rewards.sort(key=lambda t: (not t['assigned'], t['name'].lower()))
     return jsonify({
-        'rewards': rewards,
-        'rewards_count': len(rewards),
+        'rewards': result_rewards,
+        'rewards_count': len(result_rewards),
         'list_type': 'reward'
     }), 200
 
@@ -511,18 +543,26 @@ def list_assignable_rewards(id):
     child = result[0]
     assigned_ids = set(child.get('rewards', []))
 
-    all_reward_ids = [r.get('id') for r in reward_db.all() if r and r.get('id')]
-    assignable_ids = [rid for rid in all_reward_ids if rid not in assigned_ids]
+    # Get all assignable rewards (not already assigned)
+    all_rewards = [r for r in reward_db.all() if r and r.get('id') and r.get('id') not in assigned_ids]
 
-    RewardQuery = Query()
-    assignable_rewards = []
-    for rid in assignable_ids:
-        reward = reward_db.get((RewardQuery.id == rid) & ((RewardQuery.user_id == user_id) | (RewardQuery.user_id == None)))
-        if not reward:
-            continue
-        cr = ChildReward(reward.get('name'), reward.get('cost'), reward.get('image_id'), reward.get('id'))
-        assignable_rewards.append(cr.to_dict())
+    # Group by name
+    from collections import defaultdict
+    name_to_rewards = defaultdict(list)
+    for r in all_rewards:
+        name_to_rewards[r.get('name')].append(r)
 
+    filtered_rewards = []
+    for name, rewards in name_to_rewards.items():
+        user_rewards = [r for r in rewards if r.get('user_id') is not None]
+        if len(user_rewards) == 0:
+            filtered_rewards.append(rewards[0])
+        elif len(user_rewards) == 1:
+            filtered_rewards.append(user_rewards[0])
+        else:
+            filtered_rewards.extend(user_rewards)
+
+    assignable_rewards = [ChildReward(r.get('name'), r.get('cost'), r.get('image_id'), r.get('id')).to_dict() for r in filtered_rewards]
     return jsonify({'rewards': assignable_rewards, 'count': len(assignable_rewards)}), 200
 
 @child_api.route('/child/<id>/trigger-reward', methods=['POST'])
diff --git a/backend/tests/test_child_api.py b/backend/tests/test_child_api.py
index 075066c..d1bf282 100644
--- a/backend/tests/test_child_api.py
+++ b/backend/tests/test_child_api.py
@@ -270,3 +270,81 @@ def test_set_child_tasks_child_not_found(client):
     # New backend returns 400 for missing child
     assert resp.status_code in (400, 404)
     assert b'error' in resp.data
+
+def test_assignable_tasks_user_overrides_system(client):
+    """If a user task exists with the same name as a system task, only the user task should be shown in assignable list."""
+    child_db.truncate()
+    task_db.truncate()
+    # System task (user_id=None)
+    task_db.insert({'id': 'sys1', 'name': 'Duplicate', 'points': 1, 'is_good': True, 'user_id': None})
+    # User task (same name)
+    task_db.insert({'id': 'user1', 'name': 'Duplicate', 'points': 2, 'is_good': True, 'user_id': 'testuserid'})
+    client.put('/child/add', json={'name': 'Sam', 'age': 8})
+    child_id = client.get('/child/list').get_json()['children'][0]['id']
+    resp = client.get(f'/child/{child_id}/list-assignable-tasks')
+    assert resp.status_code == 200
+    data = resp.get_json()
+    names = [t['name'] for t in data['tasks']]
+    ids = [t['id'] for t in data['tasks']]
+    # Only the user task should be present
+    assert names == ['Duplicate']
+    assert ids == ['user1']
+
+def test_assignable_tasks_multiple_user_same_name(client):
+    """If two user tasks exist with the same name as a system task, both user tasks are shown, not the system one."""
+    child_db.truncate()
+    task_db.truncate()
+    # System task (user_id=None)
+    task_db.insert({'id': 'sys1', 'name': 'Duplicate', 'points': 1, 'is_good': True, 'user_id': None})
+    # User tasks (same name, different user_ids)
+    task_db.insert({'id': 'user1', 'name': 'Duplicate', 'points': 2, 'is_good': True, 'user_id': 'testuserid'})
+    task_db.insert({'id': 'user2', 'name': 'Duplicate', 'points': 3, 'is_good': True, 'user_id': 'otheruserid'})
+    client.put('/child/add', json={'name': 'Sam', 'age': 8})
+    child_id = client.get('/child/list').get_json()['children'][0]['id']
+    resp = client.get(f'/child/{child_id}/list-assignable-tasks')
+    assert resp.status_code == 200
+    data = resp.get_json()
+    names = [t['name'] for t in data['tasks']]
+    ids = [t['id'] for t in data['tasks']]
+    # Both user tasks should be present, not the system one
+    assert set(names) == {'Duplicate'}
+    assert set(ids) == {'user1', 'user2'}
+
+def test_assignable_rewards_user_overrides_system(client):
+    """If a user reward exists with the same name as a system reward, only the user reward should be shown in assignable list."""
+    child_db.truncate()
+    reward_db.truncate()
+    # System reward (user_id=None)
+    reward_db.insert({'id': 'sysr1', 'name': 'Prize', 'cost': 5, 'user_id': None})
+    # User reward (same name)
+    reward_db.insert({'id': 'userr1', 'name': 'Prize', 'cost': 10, 'user_id': 'testuserid'})
+    client.put('/child/add', json={'name': 'Sam', 'age': 8})
+    child_id = client.get('/child/list').get_json()['children'][0]['id']
+    resp = client.get(f'/child/{child_id}/list-assignable-rewards')
+    assert resp.status_code == 200
+    data = resp.get_json()
+    names = [r['name'] for r in data['rewards']]
+    ids = [r['id'] for r in data['rewards']]
+    # Only the user reward should be present
+    assert names == ['Prize']
+    assert ids == ['userr1']
+
+def test_assignable_rewards_multiple_user_same_name(client):
+    """If two user rewards exist with the same name as a system reward, both user rewards are shown, not the system one."""
+    child_db.truncate()
+    reward_db.truncate()
+    # System reward (user_id=None)
+    reward_db.insert({'id': 'sysr1', 'name': 'Prize', 'cost': 5, 'user_id': None})
+    # User rewards (same name, different user_ids)
+    reward_db.insert({'id': 'userr1', 'name': 'Prize', 'cost': 10, 'user_id': 'testuserid'})
+    reward_db.insert({'id': 'userr2', 'name': 'Prize', 'cost': 15, 'user_id': 'otheruserid'})
+    client.put('/child/add', json={'name': 'Sam', 'age': 8})
+    child_id = client.get('/child/list').get_json()['children'][0]['id']
+    resp = client.get(f'/child/{child_id}/list-assignable-rewards')
+    assert resp.status_code == 200
+    data = resp.get_json()
+    names = [r['name'] for r in data['rewards']]
+    ids = [r['id'] for r in data['rewards']]
+    # Both user rewards should be present, not the system one
+    assert set(names) == {'Prize'}
+    assert set(ids) == {'userr1', 'userr2'}
\ No newline at end of file