Skip to content
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

GraphQL - Post Lock #875

Merged
merged 27 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
64295f7
add lock unlock migration
toyhammered Oct 30, 2020
2e27bce
add lock_reason enum
toyhammered Oct 30, 2020
4d0e7e7
update post resource
toyhammered Oct 30, 2020
6149464
update post permissions
toyhammered Oct 30, 2020
f3fa15b
update comment policy for locked unlocked
toyhammered Oct 30, 2020
36b3c97
update migration to only include locked
toyhammered Oct 31, 2020
7aef6ec
update post factory
toyhammered Oct 31, 2020
203338d
remove fields
toyhammered Oct 31, 2020
b7611c7
add locked? method
toyhammered Oct 31, 2020
c4b3bad
update migration
toyhammered Oct 31, 2020
5f1b240
update graphql post type
toyhammered Oct 31, 2020
dee308e
update post model
toyhammered Oct 31, 2020
28e9641
update base input to include current_user
toyhammered Oct 31, 2020
72555ae
add spec for post model
toyhammered Oct 31, 2020
737f14a
add post lock and unlock
toyhammered Oct 31, 2020
127d606
rename all to locked_reason
toyhammered Nov 1, 2020
19c788d
add update_lock permission
toyhammered Nov 1, 2020
8825662
update post policy spec
toyhammered Nov 1, 2020
6d75e95
idk what happened...
toyhammered Nov 1, 2020
84ee233
update to use new roles
toyhammered Nov 1, 2020
7fde5b4
Delete .ruby-version
toyhammered Nov 1, 2020
d46a850
Update app/policies/post_policy.rb
toyhammered Nov 2, 2020
4c9d42c
update to locked_reason enum
toyhammered Nov 2, 2020
3c3d98a
Merge branch 'feature/post-locking' of github.com:hummingbird-me/kits…
toyhammered Nov 2, 2020
c6d1064
fix post factory
toyhammered Nov 2, 2020
915fdee
add addition lock reason
toyhammered Nov 6, 2020
5e9c031
allow owners to lock own post
toyhammered Nov 6, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions app/graphql/mutations/post/lock_post.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
class Mutations::Post::LockPost < Mutations::Base
argument :input,
Types::Input::Post::Lock,
required: true,
description: 'Lock a Post.',
as: :post

field :post, Types::Post, null: true

def load_post(value)
post = ::Post.find(value.id)
post.assign_attributes(value.to_model)
post
end

def authorized?(post:)
super(post, :update_lock?)
end

def resolve(post:)
post.save

if post.errors.any?
Errors::RailsModel.graphql_error(post)
else
{
post: post
}
end
end
end
31 changes: 31 additions & 0 deletions app/graphql/mutations/post/unlock_post.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
class Mutations::Post::UnlockPost < Mutations::Base
argument :input,
Types::Input::Post::Unlock,
required: true,
description: 'Unlock a Post.',
as: :post

field :post, Types::Post, null: true

def load_post(value)
post = ::Post.find(value.id)
post.assign_attributes(value.to_model)
post
Comment on lines +11 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm distinctly not a fan of this thing where you get the changes from some other file and then shove them in over here... It seems like indirection without purpose

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny enough this is how it was originally done (by Matt). I liked it so I kinda just started using it also lol.

end

def authorized?(post:)
super(post, :update_lock?)
end

def resolve(post:)
post.save

if post.errors.any?
Errors::RailsModel.graphql_error(post)
else
{
post: post
}
end
Comment on lines +21 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I added an Errors::ActiveRecord::RecordInvalid in #880 that should do this for us in future with post.save!

We can change it once both are merged

end
end
4 changes: 4 additions & 0 deletions app/graphql/types/enum/lock_reason.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class Types::Enum::LockReason < Types::Enum::Base
value 'SPAM'
value 'TOO_HEATED'
end
4 changes: 4 additions & 0 deletions app/graphql/types/input/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,8 @@ def self.default_graphql_name
def to_model
to_h
end

def current_user
User.current
end
end
8 changes: 8 additions & 0 deletions app/graphql/types/input/post/lock.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class Types::Input::Post::Lock < Types::Input::Base
argument :id, ID, required: true
argument :locked_reason, Types::Enum::LockReason, required: true

def to_model
to_h.merge(locked_at: DateTime.current, locked_by: current_user)
end
Comment on lines +5 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels way too magical for me

Copy link
Member Author

@toyhammered toyhammered Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kyubey

end
7 changes: 7 additions & 0 deletions app/graphql/types/input/post/unlock.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Types::Input::Post::Unlock < Types::Input::Base
argument :id, ID, required: true

def to_model
to_h.merge(locked_at: nil, locked_by: nil, locked_reason: nil)
end
end
1 change: 1 addition & 0 deletions app/graphql/types/mutation_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ class Types::MutationType < Types::BaseObject
field :episode, Types::Mutations::EpisodeMutation, null: true
field :library_entry, Types::Mutations::LibraryEntryMutation, null: true
field :mapping, Types::Mutations::MappingMutation, null: true
field :post, Types::Mutations::PostMutation, null: true
end
9 changes: 9 additions & 0 deletions app/graphql/types/mutations/post_mutation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class Types::Mutations::PostMutation < Types::BaseObject
field :lock,
mutation: ::Mutations::Post::LockPost,
description: 'Lock a Post.'

field :unlock,
mutation: ::Mutations::Post::UnlockPost,
description: 'Unlock a Post.'
end
12 changes: 12 additions & 0 deletions app/graphql/types/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ class Types::Post < Types::BaseObject
null: false,
description: 'Html formatted content.'

field :locked_by, Types::Profile,
null: true,
description: 'The user who locked this post.'

field :locked_at, GraphQL::Types::ISO8601DateTime,
null: true,
description: 'When this post was locked.'

field :locked_reason, Types::Enum::LockReason,
null: true,
description: 'The reason why this post was locked.'

field :comments, Types::Comment.connection_type,
null: false,
description: 'All comments related to this post.'
Expand Down
6 changes: 6 additions & 0 deletions app/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ class Post < ApplicationRecord
update_algolia 'AlgoliaPostsIndex'
embed_links_in :content, to: :embed

enum locked_reason: { SPAM: 0, TOO_HEATED: 1 }
toyhammered marked this conversation as resolved.
Show resolved Hide resolved
belongs_to :user, required: true
belongs_to :edited_by, class_name: 'User'
belongs_to :target_user, class_name: 'User'
belongs_to :target_group, class_name: 'Group'
belongs_to :media, polymorphic: true
belongs_to :spoiled_unit, polymorphic: true
belongs_to :community_recommendation
belongs_to :locked_by, class_name: 'User'
has_many :post_likes, dependent: :destroy
has_many :post_follows, dependent: :destroy
has_many :comments, dependent: :destroy
Expand Down Expand Up @@ -136,6 +138,10 @@ def mentioned_users
User.where(id: processed_content[:mentioned_users])
end

def locked?
locked_by.present?
end

before_save do
# Always check if the media is NSFW and try to force into NSFWness
self.nsfw = media.try(:nsfw?) || false unless nsfw
Expand Down
10 changes: 9 additions & 1 deletion app/policies/comment_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ def update?
return false unless user
return false if user.has_role?(:banned)
return true if can_administrate?
# admins are allowed to update comments on locked posts
return false if record.post.locked?
return true if group && has_group_permission?(:content)
is_owner?
end
Expand All @@ -19,7 +21,13 @@ def create?
return false if banned_from_group?
return false if group.closed? && !member?
end
is_owner?

# admins are allowed to create comments on locked posts
if record.post.locked?
is_owner? && can_administrate?
else
is_owner?
end
end

def destroy?
Expand Down
8 changes: 8 additions & 0 deletions app/policies/post_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def update?
return false unless user
return false if user.has_role?(:banned)
return true if can_administrate?
return false if record.locked?
return true if group && has_group_permission?(:content)
is_owner?
end
Expand Down Expand Up @@ -36,6 +37,13 @@ def group
record.target_group
end

def update_lock?
return true if can_administrate?
return true if group && has_group_permission(:content)
toyhammered marked this conversation as resolved.
Show resolved Hide resolved

false
end

class Scope < Scope
def resolve
return scope if can_administrate?
Expand Down
13 changes: 12 additions & 1 deletion app/resources/post_resource.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
class PostResource < BaseResource
caching
IMMUTABLE_FIELDS = %i[locked_by locked_at locked_reason].freeze

attributes :content, :content_formatted, :comments_count, :post_likes_count,
:spoiler, :nsfw, :blocked, :deleted_at, :top_level_comments_count,
:edited_at, :target_interest, :embed, :embed_url

attributes(*IMMUTABLE_FIELDS)

has_one :user
has_one :target_user
has_one :target_group
Expand All @@ -15,11 +18,19 @@ class PostResource < BaseResource
has_many :comments
has_many :uploads

def self.creatable_fields(context)
super - IMMUTABLE_FIELDS
end

def self.updatable_fields(context)
super - IMMUTABLE_FIELDS
end

def target_interest=(val)
_model.target_interest = val.underscore.classify
end

def target_interest
_model.target_interest.underscore.dasherize if _model.target_interest
_model&.target_interest&.underscore&.dasherize
end
end
8 changes: 8 additions & 0 deletions db/migrate/20201030020815_add_lock_unlock_to_post.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddLockUnlockToPost < ActiveRecord::Migration[5.1]
def change
add_column :posts, :locked_by_id, :integer
add_column :posts, :locked_at, :datetime
add_column :posts, :locked_reason, :integer
add_index :posts, :locked_by_id
end
end
6 changes: 5 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20200822214657) do
ActiveRecord::Schema.define(version: 20201030020815) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -1216,9 +1216,13 @@
t.integer "community_recommendation_id"
t.string "ao_id"
t.integer "edited_by_id"
t.integer "locked_by_id"
t.datetime "locked_at"
t.integer "locked_reason"
t.index ["ao_id"], name: "index_posts_on_ao_id", unique: true
t.index ["community_recommendation_id"], name: "index_posts_on_community_recommendation_id"
t.index ["deleted_at"], name: "index_posts_on_deleted_at"
t.index ["locked_by_id"], name: "index_posts_on_locked_by_id"
t.index ["media_type", "media_id"], name: "posts_media_type_media_id_idx"
t.index ["target_group_id"], name: "posts_target_group_id_idx"
t.index ["target_user_id"], name: "posts_target_user_id_idx"
Expand Down
6 changes: 6 additions & 0 deletions spec/factories/posts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,11 @@
factory :post do
user
content { Faker::Lorem.sentence }

trait :locked do
association :locked_by, factory: :user, strategy: :build
locked_at { DateTime.now }
locked_reason { :SPAM }
end
end
end
1 change: 1 addition & 0 deletions spec/models/post_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
it { should belong_to(:target_user).class_name('User') }
it { should belong_to(:media) }
it { should belong_to(:spoiled_unit) }
it { should belong_to(:locked_by).class_name('User') }
it { should have_many(:post_likes).dependent(:destroy) }
it { should have_many(:comments).dependent(:destroy) }
it { should validate_length_of(:content).is_at_most(9_000) }
Expand Down
17 changes: 17 additions & 0 deletions spec/policies/comment_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,30 @@
it('should allow community mod') { should permit(community_mod, comment) }
it('should not allow other users') { should_not permit(other, comment) }
it('should not allow anons') { should_not permit(nil, comment) }

context 'when post is locked' do
let(:post) { build(:post, :locked, user: owner.resource_owner) }
let(:comment) { build(:comment, user: community_mod.resource_owner, post: post) }

it('should not allow a regular user') { should_not permit(other, comment) }
it('should allow a community_mod') { should permit(community_mod, comment) }
end
end

permissions :create? do
it('should allow owner') { should permit(owner, comment) }
it('should not allow community mod') { should_not permit(community_mod, comment) }
it('should not allow random dude') { should_not permit(other, comment) }
it('should not allow anon') { should_not permit(nil, comment) }

context 'when post is locked' do
let(:post) { build(:post, :locked, user: owner.resource_owner) }
let(:community_mod_comment) { build(:comment, user: community_mod.resource_owner, post: post) }
toyhammered marked this conversation as resolved.
Show resolved Hide resolved
let(:owner_comment) { build(:comment, user: owner.resource_owner, post: post) }

it('should not allow regular owner') { should_not permit(owner, owner_comment) }
it('should only allow community_mod') { should permit(community_mod, community_mod_comment) }
end
end

permissions :destroy? do
Expand Down
14 changes: 14 additions & 0 deletions spec/policies/post_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@
it('should allow community mod') { should permit(community_mod, post) }
it('should not allow other users') { should_not permit(other, post) }
it('should not allow anons') { should_not permit(nil, post) }

context 'when post is locked' do
let(:post) { build(:post, :locked, user: owner.resource_owner) }

it('should not allow regular user') { should_not permit(owner, post) }
it('should allow community_mod') { should permit(community_mod, post) }
end
end

permissions :update_lock? do
let(:post) { build(:post, :locked, user: owner.resource_owner) }

it('should allow community_mod') { should permit(community_mod, post) }
it('should not allow owner') { should_not permit(owner, post) }
end

permissions :create? do
Expand Down