Added can_comment and comment_ban fields to members schema #725
Conversation
rob-ghost
force-pushed
the
add-can-comment-to-members-schema
branch
from
e5a9100 to
8a50e18
Compare
WalkthroughThe pull request adds comment-related fields to member schemas and a new comment-bans schema: Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rob-ghost
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/admin-api-schema/lib/schemas/members-edit.json (1)
57-76: Consider adding validation constraints to thecomment_banfield.The
comment_banschema currently lacks validation constraints that could improve data quality:
reasonfield (line 63-64): NomaxLengthconstraint. Consider adding a reasonable limit (e.g., 500 or 2000 characters) to prevent extremely long ban reasons.
expires_atfield (line 66-68): Noformatconstraint. If this is meant to be a date-time value, consider adding"format": "date-time"to ensure valid ISO 8601 timestamps are provided.💡 Proposed validation improvements
"reason": { - "type": "string" + "type": "string", + "maxLength": 2000 }, "expires_at": { - "type": ["string", "null"] + "type": ["string", "null"], + "format": "date-time" }Note: The
formatconstraint will only apply whenexpires_atis a string, not when it'snull.packages/admin-api-schema/lib/schemas/members.json (1)
51-73: Consider adding validation constraints to thecomment_banfield.Similar to the members-edit schema, the base member definition would benefit from validation constraints:
reasonfield (line 60-61): Add amaxLengthconstraint (e.g., 2000 characters) to prevent unreasonably long ban reasons.
expires_atfield (line 63-64): Add"format": "date-time"to ensure valid ISO 8601 timestamps when a string value is provided.These constraints will help maintain data quality and provide clearer expectations for API consumers.
💡 Proposed validation improvements
"reason": { - "type": "string" + "type": "string", + "maxLength": 2000 }, "expires_at": { - "type": ["string", "null"] + "type": ["string", "null"], + "format": "date-time" }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/admin-api-schema/lib/schemas/members-edit.jsonpackages/admin-api-schema/lib/schemas/members.jsonpackages/admin-api-schema/package.json
🧰 Additional context used
📓 Path-based instructions (1)
packages/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
All packages are published under the
@tryghostscope
Files:
packages/admin-api-schema/package.json
🧠 Learnings (1)
📚 Learning: 2026-01-06T10:55:42.630Z
Learnt from: CR
Repo: TryGhost/SDK PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T10:55:42.630Z
Learning: Applies to packages/*/package.json : All packages are published under the `tryghost` scope
Applied to files:
packages/admin-api-schema/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
packages/admin-api-schema/package.json (1)
3-3: LGTM! Appropriate version bump for new features.The minor version increment (4.5.13 → 4.6.0) correctly follows semantic versioning for adding new backward-compatible fields to the schema.
packages/admin-api-schema/lib/schemas/members-edit.json (1)
54-76: Inconsistency: PR description doesn't mention thecomment_banfield.The PR description states that only
can_commentis being added, but the actual changes introduce bothcan_commentandcomment_banfields. The commit message correctly mentions both fields, suggesting the PR description may be outdated or incomplete.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 3
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
rob-ghost
force-pushed
the
add-can-comment-to-members-schema
branch
from
8a50e18 to
55ad23b
Compare
rob-ghost
force-pushed
the
add-can-comment-to-members-schema
branch
from
55ad23b to
16654c0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #725 +/- ##
==========================================
+ Coverage 84.62% 84.74% +0.12%
==========================================
Files 33 33
Lines 3518 3546 +28
Branches 394 396 +2
==========================================
+ Hits 2977 3005 +28
Misses 536 536
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/admin-api-schema/lib/schemas/members.json (2)
51-53: Consider adding a description for documentation.The
can_commentfield structure is correct. Consider adding a"description"property to document the field's purpose and its relationship tocomment_ban.📝 Suggested enhancement
"can_comment": { + "description": "Whether the member is allowed to post comments. May be false due to a ban or other moderation action.", "type": "boolean" },
54-76: Schema structure is correct; business logic verification is out of scope for this package.The
comment_banstructure is well-designed:
- Correctly uses
oneOfto allow either a ban object ornull- Properly restricts additional properties
reasonhas sensible constraints (minLength 1, maxLength 2000)expires_atcorrectly allows both ISO 8601 timestamp strings andnullfor permanent bansOptional: Add description for better documentation
📝 Suggested enhancement
"comment_ban": { + "description": "Ban details if the member is banned from commenting, or null if not banned. When present, reason is required and expires_at can be set for temporary bans.", "oneOf": [Note: This package (
admin-api-schema) is purely for JSON Schema validation of API requests. The relationship betweencan_commentandcomment_ban—including handling of potentially inconsistent states, automatic cleanup of expired bans, and privilege restoration—is enforced by Ghost core business logic, not by this validation schema.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/admin-api-schema/lib/schemas/members-edit.jsonpackages/admin-api-schema/lib/schemas/members.jsonpackages/admin-api-schema/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/admin-api-schema/package.json
- packages/admin-api-schema/lib/schemas/members-edit.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
rob-ghost
force-pushed
the
add-can-comment-to-members-schema
branch
4 times, most recently
from
f619c92 to
ba5c092
Compare
ref https://linear.app/ghost/issue/FEA-487 This enables the Admin API to accept comment moderation fields when creating or editing members: - can_comment (boolean): indicates if member can post comments - comment_ban (object|null): stores ban metadata with reason and optional expires_at
rob-ghost
force-pushed
the
add-can-comment-to-members-schema
branch
from
ba5c092 to
1523b68
Compare
| { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "reason": { "type": "string", "minLength": 1, "maxLength": 2000 }, | ||
| "expires_at": { "type": ["string", "null"], "format": "date-time" } | ||
| }, | ||
| "required": ["reason"] | ||
| }, |
There was a problem hiding this comment.
Can this use the same "$ref": "comment_bans#/definitions/comment_ban" format used above or does it need to be duplicated?
There was a problem hiding this comment.
Good question! I tried to solve this before, but the problem is we can't reference definitions from other contexts. In this case we're within the members scope, but comment_ban is defined in comment_bans scope. So the duplication is needed.
I also explored putting comment_ban into members scope because technically it exists in /members/ but then the name of the docName referenced in the request payload needs to be changed to something like members-comment_bans so I decided to leave it under the comment_bans scope and accept the duplication.
kevinansfield
left a comment
kevinansfield
left a comment
There was a problem hiding this comment.
A couple of minor questions about removing duplication but nothing blocking 👌
Description
Adds comment moderation fields to the members schema and a dedicated schema for the comment ban endpoint, enabling the Admin API to validate comment ban requests. This supports the member comment ban feature.
Changes
comment_bans.jsonbase schema withcomment_bandefinition:reason(string, required, minLength: 1, maxLength: 2000): explanation for the banexpires_at(string|null, optional, format: date-time): ISO 8601 timestamp for temporary banscomment_bans-add.jsonschema for the/members/:id/comment-banendpointcan_comment(boolean) to member propertiescomment_banproperty to member schema (references shared definition, allows null)ref https://linear.app/ghost/issue/FEA-487
ref TryGhost/Ghost#25791
Note
Introduces comment moderation support in the Admin API schema.
comment_bans.jsonbase schema withcomment_bandefinition (reason, optionalexpires_at)comment_bans-add.jsonforcomment_bans.addendpoint and registers it inindex.jsmembers.json,members-edit.json) withcan_comment(boolean) andcomment_ban(object or null)@tryghost/admin-api-schemato4.6.0Written by Cursor Bugbot for commit 1523b68. This will update automatically on new commits. Configure here.