-
Notifications
You must be signed in to change notification settings - Fork 143
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 API: Site Permissions #880
base: the-future
Are you sure you want to change the base?
Changes from all commits
f72c6bc
ab7eb9f
32233c2
b31eccd
62ad2f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
module Accounts | ||
class SetSitePermissions < Action | ||
class UnknownPermissions < StandardError; end | ||
|
||
parameter :user, load: User, required: true | ||
parameter :permissions, required: true | ||
|
||
def call | ||
sym_permissions = permissions.map(&:to_sym) | ||
|
||
unknown_permissions = sym_permissions - User.permissions.keys | ||
raise UnknownPermissions, unknown_permissions if unknown_permissions.present? | ||
|
||
user.permissions = sym_permissions | ||
user.save! | ||
|
||
{ user: user, permissions: user.permissions } | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
module Errors | ||
module ActiveRecord | ||
class RecordInvalid | ||
def self.graphql_error(error) | ||
record = error.record | ||
errors = record.errors.map do |attribute, message| | ||
{ | ||
code: 'ValidationError', | ||
message: record.errors.full_message(attribute, message), | ||
path: ['attributes', attribute.to_s.camelize(:lower)] | ||
} | ||
end | ||
|
||
{ | ||
errors: errors | ||
} | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
class Mutations::Account::SetSitePermissions < Mutations::Base | ||
argument :input, Types::Input::Account::SetSitePermissions, | ||
required: true | ||
|
||
field :account, Types::Account, null: false | ||
|
||
def authorized?(input:) | ||
super(input.account, :set_site_permissions?) | ||
end | ||
|
||
def resolve(input:) | ||
permissions = input.permissions.map(&:to_sym) | ||
|
||
res = Accounts::SetSitePermissions.call( | ||
user: input.account, | ||
permissions: permissions | ||
) | ||
|
||
{ account: res.user } | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
class Types::Input::Account::SetSitePermissions < Types::Input::Base | ||
argument :account_id, ID, | ||
required: true, | ||
description: 'Who to grant permissions to', | ||
as: :account | ||
|
||
argument :permissions, [Types::Enum::SitePermission], | ||
required: true, | ||
description: 'The permissions to set for this user' | ||
|
||
def load_account(value) | ||
::User.find(value) | ||
end | ||
Comment on lines
+11
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want this here, or inside the mutation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, because we use it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ooooh, I see what you've done. If you want to go that route shouldn't we rename the Also unless I am thinking of this wrong from the I'll test this locally tomorrow because either this won't work at all or you are about to teach me something new (and I'm hoping it is the latter because this really opens up the possibilities). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I'm doing anything weird, I'm pretty sure I've just copied it from your stuff… my thinking is that this should run on the input type to load the record and then when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
class Types::MutationType < Types::BaseObject | ||
field :pro, Types::Mutations::ProMutation, null: false | ||
field :account, Types::Mutations::AccountMutation, null: true | ||
field :anime, Types::Mutations::AnimeMutation, null: true | ||
field :episode, Types::Mutations::EpisodeMutation, null: true | ||
field :library_entry, Types::Mutations::LibraryEntryMutation, null: true | ||
field :mapping, Types::Mutations::MappingMutation, null: true | ||
field :pro, Types::Mutations::ProMutation, null: false | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class Types::Mutations::AccountMutation < Types::BaseObject | ||
field :set_site_permissions, | ||
mutation: ::Mutations::Account::SetSitePermissions, | ||
description: 'Set the sitewide permissions for a user' | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
require 'rails_helper' | ||
|
||
RSpec.describe Accounts::SetSitePermissions do | ||
let(:user) { create(:user, permissions: %i[admin]) } | ||
|
||
context 'with valid permissions' do | ||
it 'should return the user and their new permissions' do | ||
res = described_class.call(user: user, permissions: [:community_mod]) | ||
expect(res.user.id).to eq(user.id) | ||
expect(res.permissions).to be_set(:community_mod) | ||
expect(res.permissions).not_to be_set(:admin) | ||
end | ||
end | ||
|
||
context 'with an invalid permission' do | ||
it 'should throw UnknownPermissions error' do | ||
expect { | ||
described_class.call(user: user, permissions: %i[poopy community_mod]) | ||
}.to raise_exception(Accounts::SetSitePermissions::UnknownPermissions) | ||
expect(user.reload.permissions).not_to be_set(:community_mod) | ||
end | ||
end | ||
end |
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 type of errors can happen here and should we account for them?
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.
UnknownPermissions
could happen due to a programming error but otherwise it won't happen, and the only other ones areRecordNotFound
orRecordInvalid
which I believe are accounted for automatically