Skip to content

feat(todo): categories and tags for todos (#28)#89

Open
demo5-labworksdev wants to merge 4 commits intomainfrom
feat/categories-tags
Open

feat(todo): categories and tags for todos (#28)#89
demo5-labworksdev wants to merge 4 commits intomainfrom
feat/categories-tags

Conversation

@demo5-labworksdev
Copy link
Copy Markdown

Summary

Implements issue #28 — Categories / Tags for todos.

  • Tags table with predefined categories: Work (blue), Personal (green), Shopping (orange), Health (red)
  • Multi-select tag checkboxes on the add form with color-coded badges
  • Filter by tag via ?tag=<id> query param with a filter bar on the index page
  • Strikethrough styling on completed todos

Security

  • Session-based ownership: todos are scoped to the browser session that created them
  • CSRF tokens on all mutating forms (POST-only toggle/delete)
  • Role-based authorization via _ROLE_PERMISSIONS map; session role validated against known roles
  • owner_id format validated against ^[0-9a-f]{32}$ before any DB operation
  • Secure session cookie attributes: HttpOnly, SameSite=Lax, Secure (controlled via HTTPS env var)
  • Tag IDs submitted with new todos are validated against the system tags table

Test plan

  • 24 unit tests pass, 100% coverage (uv run pytest tests/test_app.py -v)
  • Add a todo with multiple tags and verify badges appear
  • Click a tag filter and verify only matching todos are shown
  • Toggle and delete work correctly for own todos
  • Verify toggle/delete return 403 for another session's todos

🤖 Generated with Claude Code

- Add tags and todo_tags tables with predefined categories: Work, Personal, Shopping, Health
- Multi-select tag checkboxes on the add form with color-coded badges
- Filter todos by tag via ?tag=<id> query param
- Session-based ownership: todos are scoped to the creating browser session
- CSRF protection on all mutating endpoints (POST-only toggle/delete)
- Role-based authorization via _ROLE_PERMISSIONS with session role validation
- owner_id format validated against ^[0-9a-f]{32}$ to prevent malformed values
- Secure session cookie attributes: HttpOnly, SameSite=Lax, Secure (env-controlled)
- 24 unit tests, 100% coverage

Closes #28

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@ai-coding-guardrails ai-coding-guardrails bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got 1 comment for you to consider

Reviewed with 🤟 by Zenable


# Switch session to a different owner
with client.session_transaction() as sess:
sess["owner_id"] = "different-owner"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The owner_id value "different-owner" does not match the ^[0-9a-f]{32}$ format that the PR description states is validated before any DB operation. If the app validates owner_id format and rejects or ignores malformed values, this test may be passing for the wrong reason — the 403 could be returned due to format validation failure rather than ownership enforcement. The same issue exists in test_delete_forbidden_for_non_owner (line 274). Use a valid 32-char hex string (e.g., "b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5") to ensure the 403 is actually testing ownership authorization and not input validation.

Why did I show this?

Category: bug
Comment Quality: high

Influenced by requirements:

Tools used:

  1. get_file_lines, {'file_path': 'todo_app/app.py', 'start_line': '1', 'end_line': '200'}
  2. find_files, {'pattern': '**/app.py'}

demo5-labworksdev and others added 3 commits April 7, 2026 13:03
Pass current_owner_id to template and guard action buttons with
{% if todo.owner_id == current_owner_id %} so users never see
controls for todos they do not own.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d format

- Set todo['is_owner'] in index route (server-side) so template uses a boolean
  flag rather than comparing raw owner IDs
- Validate the DB-stored owner_id matches ^[0-9a-f]{32}$ inside authorize_todo
  as defence-in-depth against malformed records

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant