feat: add goal meta support with size validation#132
feat: add goal meta support with size validation#132devbeoservice wants to merge 3 commits intocjs8487:mainfrom
Conversation
devbeoservice
commented
Sep 20, 2025
devbeoservice
commented
- Add meta Json? field to Goal model in Prisma schema
- Create migration with 4096 character constraint using JSONB
- Add GoalValidation utility for application-level validation
- Integrate meta validation into Goals API endpoint
- Update Goal.json schema to include meta field
- Support auto-tracking keys and flexible metadata storage
- Add meta Json? field to Goal model in Prisma schema - Create migration with 4096 character constraint using JSONB - Add GoalValidation utility for application-level validation - Integrate meta validation into Goals API endpoint - Update Goal.json schema to include meta field - Support auto-tracking keys and flexible metadata storage
cjs8487
left a comment
cjs8487
left a comment
There was a problem hiding this comment.
Overall this looks pretty good. Ideally we would get this into the editor as well but it’s not a huge deal right now, especially while we try and work out the exact details of the client side json editor. One request outside of the direct comments - you need to run the generate script in the schema package to generate the .d.ts files used by the client and server
api/src/util/GoalValidation.ts
Outdated
| * Gets the maximum allowed size for goal meta data | ||
| * @returns Maximum size in characters | ||
| */ | ||
| export function getGoalMetaMaxSize(): number { |
There was a problem hiding this comment.
Is there a practical use case for this function? Do we anticipate a need to modify the constant in some way to necessitate wrapping it like this? If not, I’d probably lean towards just exporting the constant directly, if we even need to export the value at all
There was a problem hiding this comment.
I've thought about a config table inside the database before, then we could've reused that in some init script to set this constant and thats why I had it seperate, since I didn't opt for that it's probably best to remove the constant and hardcode the number.
There was a problem hiding this comment.
added jsdocs because its security related code, added 20 max goals with max 5 nested. earlier it would have full ignored this because its not checking against json schema in the api. is this on purpose?
Also kept the constants because it would otherwise look like magic numbers for some.
There was a problem hiding this comment.
I’m fine with keeping the constant, but I still don’t think the function is necessary. If you’re going to export a function that just returns a constant, why not just return the constant directly? I can’t see a use case where we would want to do an operation on the constant before returning it, which is the only case where I could see the function being sensible
|
I'm inclined to think that we should put merging this on hold until we merge the json editor in. The editor is pretty close at this point I think, and it'd be a pretty big win to be able to edit these out of the box without risking it getting split across multiple releases |
cjs8487
added
enhancement
cjs8487
removed
the
waiting for release
2 participants
