-
Notifications
You must be signed in to change notification settings - Fork 23
Issue 73: Implement tagging for queries #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Issue 73: Implement tagging for queries #121
Conversation
| owner_id TEXT NOT NULL, | ||
| active INTEGER NOT NULL, | ||
| description TEXT NOT NULL, | ||
| tags TEXT DEFAULT '[]', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a new table with tags linked to queries instead of adding to the queries table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback!
I will update the implementation to match the expected design:
Next Steps I Will Implement
Create a separate query_tags table:
id
query_id (foreign key)
type
value
Remove the tags column from the main queries table.
Update:
Repository layer (to insert, update, delete tags)
QueryService
QueryHandler
API responses (to return tags joined from the new table)
This aligns with the intended data model for issue #73.
I’ll push the updated changes shortly. Thanks for the review.
| @@ -0,0 +1,38 @@ | |||
| package main | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right to point out that adding a separate cmd/query_api/main.go ended up duplicating functionality that already exists in backend/server.go. I originally added it as a small standalone server to test the Queries feature during development, but I agree it does not fit the project’s structure.
Here is what I will update:
Remove the entire cmd/query_api/main.go file.
Move all routing logic for Queries into the existing server by registering the routes inside backend/routes.go along with the existing queries and api endpoints.
Make sure all Query handlers run through the main backend server in backend/server.go instead of using a separate entry point.
I will push an updated commit with these changes.
| "encoding/json" | ||
| "net/http" | ||
|
|
||
| "github.com/gorilla/mux" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why switch to gorilla/mux from julienschmidt/httprouter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right, switching to gorilla/mux was unnecessary.
I'll revert this change and keep using julienschmidt/httprouter to stay consistent with the existing backend architecture.
I initially added mux because I created a temporary standalone query_api server for local testing during development, but since this is not needed, I'll remove that file and integrate everything into the existing server.go + routes.go structure that already uses httprouter.
Add tag support for saved queries (Issue #73)
Summary:
Implemented tag support for saved queries as described in Issue #73.
Changes:
Result:
Users can now:
This completes the backend portion required for Issue #73.