-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add CommentPolicy and CommentThreadPolicy (part 2 of Commontator removal) #7835
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
Open
Shruti2110-coder
wants to merge
7
commits into
CircuitVerse:master
Choose a base branch
from
Shruti2110-coder:feat/comment-policy
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2c526ab
feat: add native Comment and CommentThread models
Shruti2110-coder f1034d3
fix: reject cyclic reply parents
Shruti2110-coder e44ee07
fix: restore has_many :submissions dropped during rebase
Shruti2110-coder 3f7991b
docs: note that the cycle guard is application-level only
Shruti2110-coder fcad6c3
feat: add CommentPolicy and CommentThreadPolicy
Shruti2110-coder b01f5c2
fix: define restore? explicitly rather than delegating to destroy?
Shruti2110-coder 5dcd4bc
fix: require a deleted comment before authorizing restore
Shruti2110-coder File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Mirrors Commontator::Comment. Belongs to a thread rather than directly to the | ||
| # commentable, so that thread level state (open/closed, subscriptions) has a | ||
| # home and the existing API shape is preserved. | ||
| class Comment < ApplicationRecord | ||
| BODY_MAX_LENGTH = 10_000 | ||
|
|
||
| belongs_to :comment_thread, inverse_of: :comments | ||
| belongs_to :user | ||
| belongs_to :editor, class_name: "User", optional: true | ||
| belongs_to :parent, class_name: "Comment", optional: true | ||
|
|
||
| has_many :replies, class_name: "Comment", | ||
| foreign_key: :parent_id, | ||
| inverse_of: :parent, | ||
| dependent: :destroy | ||
|
|
||
| validates :body, presence: true, length: { maximum: BODY_MAX_LENGTH } | ||
| validate :parent_shares_thread | ||
| validate :parent_is_not_cyclic | ||
|
|
||
| scope :kept, -> { where(deleted_at: nil) } | ||
| scope :roots, -> { where(parent_id: nil) } | ||
| scope :chronological, -> { order(created_at: :asc) } | ||
|
|
||
| def deleted? | ||
| deleted_at.present? | ||
| end | ||
|
|
||
| def edited? | ||
| editor_id.present? | ||
| end | ||
|
|
||
| def soft_delete! | ||
| update!(deleted_at: Time.current) | ||
| end | ||
|
|
||
| def restore! | ||
| update!(deleted_at: nil) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # A reply must live in the same thread as its parent, otherwise a comment | ||
| # could be threaded onto a discussion it does not belong to. | ||
| def parent_shares_thread | ||
| return if parent.nil? | ||
| return if parent.comment_thread_id == comment_thread_id | ||
|
|
||
| errors.add(:parent, "must belong to the same thread") | ||
| end | ||
|
|
||
| # Walk up the ancestor chain so a comment cannot be its own parent, nor | ||
| # part of a reply cycle. Without this, `replies` recurses forever during | ||
| # dependent: :destroy. | ||
| # | ||
| # NOTE: application-level guard only. It does not run for update_column, | ||
| # insert_all, or raw SQL, and does not protect against concurrent writes. | ||
| # The invariant must be re-checked wherever rows are written outside the | ||
| # model, in particular the Commontator data migration. | ||
| def parent_is_not_cyclic | ||
| return if parent.nil? | ||
|
|
||
| if parent == self | ||
| errors.add(:parent, "cannot be the comment itself") | ||
| return | ||
| end | ||
|
|
||
| # A new record has no id, so nothing can point back at it yet. | ||
| return if id.nil? | ||
|
|
||
| ancestor = parent | ||
| while ancestor | ||
| if ancestor.parent_id == id | ||
| errors.add(:parent, "cannot create a cycle") | ||
| return | ||
| end | ||
|
|
||
| ancestor = ancestor.parent | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Mirrors Commontator::Thread. One thread per commentable, holding the comments | ||
| # and the open/closed state. | ||
| class CommentThread < ApplicationRecord | ||
| belongs_to :commentable, polymorphic: true | ||
| belongs_to :closer, class_name: "User", optional: true | ||
|
|
||
| has_many :comments, dependent: :destroy | ||
|
|
||
| def closed? | ||
| closed_at.present? | ||
| end | ||
|
|
||
| def close!(user) | ||
| update!(closed_at: Time.current, closer: user) | ||
| end | ||
|
|
||
| def reopen! | ||
| update!(closed_at: nil, closer: nil) | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Mirrors the comment permissions in commontator-7.0.1, as configured in | ||
| # config/initializers/commontator.rb: | ||
| # | ||
| # comment_editing = :a -> authors may always edit their own comments | ||
| # comment_deletion = :a -> authors may always delete their own comments | ||
| # moderator_permissions = :d -> moderators may delete and close, not edit | ||
| # comment_voting = :ld -> likes and dislikes are both enabled | ||
| # | ||
| # Intentionally does not call super, for the same reason as | ||
| # CommentThreadPolicy: comments on public projects are readable anonymously. | ||
| class CommentPolicy < ApplicationPolicy | ||
| attr_reader :user, :comment | ||
|
|
||
| delegate :show?, to: :thread_policy | ||
|
|
||
| def initialize(user, comment) | ||
| @user = user | ||
| @comment = comment | ||
| end | ||
|
|
||
| def create? | ||
| thread_policy.create_comment? | ||
| end | ||
|
|
||
| # Commontator#can_be_edited_by?: moderators are excluded here because | ||
| # moderator_permissions is :d rather than :e. | ||
| def update? | ||
| author? && | ||
| !comment.deleted? && | ||
| !thread_closed? && | ||
| (comment.editor_id.nil? || comment.editor_id == user.id) && | ||
| show? | ||
| end | ||
|
|
||
| # Commontator#can_be_deleted_by?: the moderator branch returns before the | ||
| # closed-thread check, so moderators may still delete in a closed thread. | ||
| # An author may undelete only a comment they deleted themselves. | ||
| def destroy? | ||
| return true if thread_policy.moderator? | ||
|
|
||
| author? && | ||
| !thread_closed? && | ||
| (!comment.deleted? || comment.editor_id == user.id) && | ||
| show? | ||
| end | ||
|
|
||
| # Commontator routes undelete through can_be_deleted_by?, so moderators may | ||
| # restore any comment, while an author may only restore one they deleted | ||
| # themselves. | ||
| def restore? | ||
| return true if thread_policy.moderator? | ||
|
|
||
| author? && | ||
| comment.deleted? && | ||
| comment.editor_id == user.id && | ||
| !thread_closed? && | ||
| show? | ||
| end | ||
|
|
||
| # Commontator#can_be_voted_on_by?: a user may not vote on their own comment, | ||
| # and voting closes along with the thread. | ||
| def vote? | ||
| user.present? && | ||
| !author? && | ||
| !comment.deleted? && | ||
| !thread_closed? && | ||
| show? | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def author? | ||
| user.present? && comment.user_id == user.id | ||
| end | ||
|
|
||
| def thread_closed? | ||
| comment.comment_thread&.closed? || false | ||
| end | ||
|
|
||
| def thread_policy | ||
| @thread_policy ||= CommentThreadPolicy.new(user, comment.comment_thread) | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Mirrors the thread permissions Commontator is currently configured with: | ||
| # | ||
| # thread_read_proc -> public commentable, or ProjectPolicy view access | ||
| # thread_moderator_proc -> user.admin? | ||
| # | ||
| # Intentionally does not call super: threads on public projects must stay | ||
| # readable by anonymous users, matching the current API behaviour where | ||
| # unauthenticated requests can list a public project's comments. | ||
| class CommentThreadPolicy < ApplicationPolicy | ||
| attr_reader :user, :comment_thread | ||
|
|
||
| def initialize(user, comment_thread) | ||
| @user = user | ||
| @comment_thread = comment_thread | ||
| end | ||
|
|
||
| def show? | ||
| readable? | ||
| end | ||
|
|
||
| def create_comment? | ||
| user.present? && !comment_thread.closed? && readable? | ||
| end | ||
|
|
||
| def subscribe? | ||
| user.present? && readable? | ||
| end | ||
|
|
||
| def unsubscribe? | ||
| subscribe? | ||
| end | ||
|
|
||
| def close? | ||
| moderator? | ||
| end | ||
|
|
||
| def reopen? | ||
| moderator? | ||
| end | ||
|
|
||
| # moderator_permissions is :d, so moderators may delete comments and close | ||
| # threads, but may not edit other people's comments. | ||
| def moderator? | ||
| user.present? && user.admin? | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def readable? | ||
| commentable = comment_thread.commentable | ||
| return false if commentable.nil? | ||
| return true if commentable.public? | ||
|
|
||
| ProjectPolicy.new(user, commentable).check_view_access? | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class CreateCommentTables < ActiveRecord::Migration[8.1] | ||
| def change | ||
| create_comment_threads | ||
| create_comments | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def create_comment_threads | ||
| create_table :comment_threads do |t| | ||
| t.references :commentable, polymorphic: true, null: false, index: false | ||
| t.datetime :closed_at | ||
| t.references :closer, null: true, foreign_key: { to_table: :users } | ||
|
|
||
| t.timestamps | ||
| end | ||
|
|
||
| add_index :comment_threads, %i[commentable_type commentable_id], | ||
| unique: true, | ||
| name: "index_comment_threads_on_commentable" | ||
| end | ||
|
|
||
| def create_comments | ||
| create_table :comments do |t| | ||
| t.references :comment_thread, null: false, foreign_key: true, index: false | ||
| t.references :user, null: false, foreign_key: true | ||
| t.references :editor, null: true, foreign_key: { to_table: :users } | ||
| t.references :parent, null: true, foreign_key: { to_table: :comments } | ||
| t.text :body, null: false | ||
| t.datetime :deleted_at | ||
|
|
||
| t.timestamps | ||
| end | ||
|
|
||
| add_index :comments, %i[comment_thread_id created_at] | ||
| add_index :comments, :deleted_at | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| FactoryBot.define do | ||
| factory :comment_thread do | ||
| association :commentable, factory: :project | ||
|
|
||
| trait :closed do | ||
| closed_at { Time.current } | ||
| association :closer, factory: :user | ||
| end | ||
| end | ||
|
|
||
| factory :comment do | ||
| association :comment_thread | ||
| association :user | ||
| body { "A comment body" } | ||
|
|
||
| trait :deleted do | ||
| deleted_at { Time.current } | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.