Skip to content

feat(core): add label CRUD service - #22

Merged
dylany3 merged 3 commits into
mainfrom
dylany/label-crud
Oct 5, 2026
Merged

dylany3 merged 3 commits into
mainfrom
dylany/label-crud

Conversation

@dylany3

@dylany3 dylany3 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #14

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Adds a Label service (packages/core/src/label.ts) with create, get, list, update, and remove for labels. Built on top of #21 (label table), so the base is brianz2/database-labels until that merges.

Invalid operations fail with typed errors instead of doing something weird:

  • Editing or deleting a nonexistent/already deleted label: NotFoundError
  • Empty or too-long name (over 64 chars): InvalidNameError
  • Duplicate name under the same parent (case-insensitive): DuplicateNameError
  • Moving a label under itself or its own child: CycleError

Deleting a label also deletes its child labels and returns all deleted IDs, so #15 can clean up session assignments. Open to feedback on this and on requiring unique sibling names.

How did you verify your code works?

Added packages/core/test/label.test.ts with 18 tests covering each operation and every invalid case above. Run with bun test test/label.test.ts in packages/core; all pass. Typecheck and Prettier also pass.

Screenshots / recordings

N/A, backend only

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@dylany3 dylany3 self-assigned this Sep 25, 2026
@dylany3 dylany3 linked an issue Sep 25, 2026 that may be closed by this pull request
@jahuang24

jahuang24 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

my claude found a mistake.

Issue: {"parentID": ""} (an empty string) returns a 500 instead of a 404. The service treats "" as "no parent", skips checking that the parent exists, and then inserts parent_id = "", which fails the database's foreign key check.
Possible Solution: Change Label.ID schema to reject empty strings, which would make it a clean 400.

u just got claudemogged

@Yumeseeker Yumeseeker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For create and update, if the parentID is "", it could bypass the requireRow check. maybe make code explicit like if (parentID !== null)...

Shouldn't matter for users since all created labels are validated, but could be an edge case for internal use?

@dylany3

dylany3 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

@jahuang24 @Yumeseeker thanks for catching this! Fixed in ccc8550: create and update now check parentID !== null instead of truthiness, so "" goes through requireRow and returns NotFoundError instead of hitting the DB foreign key. Added a regression test for both create and update (19 tests passing, CI green).

@dylany3
dylany3 changed the base branch from brianz2/database-labels to main October 5, 2026 21:08
@dylany3
dylany3 merged commit cd9798a into main Oct 5, 2026
6 checks passed
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.

CRUD Operations for labels

3 participants