Add CoPlan::Tag model with join table, API, and admin#79
Add CoPlan::Tag model with join table, API, and admin#79HamptonMakes wants to merge 1 commit intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f2c553176
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| module V1 | ||
| class TagsController < BaseController | ||
| def index | ||
| tags = CoPlan::Tag.order(plans_count: :desc, name: :asc) |
There was a problem hiding this comment.
GET /api/v1/tags reads all tags via CoPlan::Tag.order(...) and returns global plans_count values without applying any plan visibility filter, so any authenticated token can discover tags attached to plans they should not be able to enumerate (including brainstorm-only content that Api::V1::PlansController#index intentionally hides). Scope the tags/counts to plans the current user can access before returning them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — tags index now scopes to visible plans only (non-brainstorm + user's own brainstorm), matching PlansController#index visibility. Added specs for both the exclusion and inclusion cases. The plans_count returned is the visible count, not the global counter cache.
| t.index ["tag_id"], name: "index_coplan_plan_tags_on_tag_id" | ||
| end | ||
|
|
||
| create_table "coplan_plan_types", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| |
There was a problem hiding this comment.
db/schema.rb in this commit introduces coplan_plan_types, coplan_plans.plan_type_id, and related foreign keys/columns, but there are no corresponding migration files in db/migrate or engine/db/migrate for those objects. This creates migration/schema drift (schema-load environments get structures migration-based environments never create), which can hide deployment and data-integrity issues; keep schema changes limited to objects created by migrations in the same commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved — after rebasing on main, coplan_plan_types and related columns now have matching migrations from the plan-types feature that landed on main. The schema.rb is regenerated from a clean database.
HamptonMakes
force-pushed
the
tag-model
branch
7 times, most recently
from
d362eae to
17c6bb4
Compare
- Create coplan_tags table (name, plans_count counter cache) and coplan_plan_tags join table with unique constraint - Add CoPlan::Tag and CoPlan::PlanTag models with validations - Add has_many :plan_tags / :structured_tags to Plan model (keeps existing JSON tags column) - Add GET /api/v1/tags endpoint sorted by frequency - Add ActiveAdmin registration for tags - Add factories and specs for models and API Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d54e9-adf1-746a-861d-3268c9113fa0
1 participant
Summary
Introduces a first-class
CoPlan::Tagmodel to replace/supplement the existing JSONtagsarray column on plans.Changes
coplan_tagstable (name + plans_count counter cache) andcoplan_plan_tagsjoin table with unique[plan_id, tag_id]constraintCoPlan::TagandCoPlan::PlanTagwith validations, counter cache, and ransackable methodshas_many :plan_tags/has_many :structured_tags(existing JSONtagscolumn preserved)GET /api/v1/tagsreturning all tags sorted by frequency (plans_count desc)tags.rbandplan_tags.rbNotes
tagscolumn on plans is kept for backward compatibility