feat: Enhance task and reward assignment logic to prioritize user items over system items with the same name; add corresponding tests
All checks were successful
Gitea Actions Demo / build-and-push (push) Successful in 15s
All checks were successful
Gitea Actions Demo / build-and-push (push) Successful in 15s
This commit is contained in:
2
.github/copilot-instructions.md
vendored
2
.github/copilot-instructions.md
vendored
@@ -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.
|
||||
|
||||
19
.github/specs/archive/bug-both-system-and-user-items-shown.md
vendored
Normal file
19
.github/specs/archive/bug-both-system-and-user-items-shown.md
vendored
Normal file
@@ -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.
|
||||
@@ -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'])
|
||||
|
||||
@@ -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'}
|
||||
Reference in New Issue
Block a user