Favoring method aliasing over explicit delegation (#16526)

Prior to this commit many of the policy methods were of the form:

```
def create?
  edit?
end
```

The above works, but introduces a place for code drift.

We can shorten this to:

```
alias create? edit?
```

This helps keep the policy methods compact and scannable.  All things
authorization policy related should create narrow surface areas.  By
hand-crafting methods, it is easier for "slippage" to occur.

Another advantage of `alias` is it's use is reflected in API
documentation.  With alias you need only write documentinig comments for
one method and the doc generators will relate the alias and the original
method.

If you "hand craft" a method using explicit delegation, the API
documentation won't pick up that two methods are aliases.

Further, I believe we should be moving towards parity between the
GET/POST/PUT methods that are related (e.g. the create/new pair and the
update/edit pair).  This is not the case, as we "silently" suspend
folks.  As we move through the [authorization work][1], we will start
hiding the links/buttons when a user can't take the action.

Related to #16483 and #16523

[1]:https://github.com/orgs/forem/projects/46
This commit is contained in:
Jeremy Friesen 2022-02-10 11:09:17 -05:00 committed by GitHub
parent 89706527b0
commit 87e1622e6b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 48 additions and 130 deletions

View file

@ -28,25 +28,17 @@ class ArticlePolicy < ApplicationPolicy
!user_suspended?
end
def delete_confirm?
update?
end
alias delete_confirm? update?
def discussion_lock_confirm?
update?
end
alias discussion_lock_confirm? update?
def discussion_unlock_confirm?
update?
end
alias discussion_unlock_confirm? update?
def destroy?
update?
end
alias destroy? update?
def preview?
true
end
alias edit? update?
alias preview? new?
def stats?
user_author? || user_admin? || user_org_admin?

View file

@ -7,21 +7,15 @@ class CommentPolicy < ApplicationPolicy
!user_suspended? && !user.comment_suspended?
end
def update?
edit?
end
alias new? create?
def destroy?
edit?
end
alias update? edit?
def delete_confirm?
edit?
end
alias destroy? edit?
def settings?
edit?
end
alias delete_confirm? edit?
alias settings? edit?
def preview?
true
@ -35,9 +29,7 @@ class CommentPolicy < ApplicationPolicy
user_commentable_author?
end
def unhide?
user_commentable_author?
end
alias unhide? hide?
def admin_delete?
minimal_admin?

View file

@ -7,15 +7,9 @@ class ConsumerAppPolicy < ApplicationPolicy
@record.creator_app?
end
def edit?
@record.creator_app?
end
alias edit? create?
def update?
@record.creator_app?
end
alias update? create?
def destroy?
@record.creator_app?
end
alias destroy? create?
end

View file

@ -5,9 +5,7 @@ class DiscussionLockPolicy < ApplicationPolicy
(user_author? || minimal_admin?) && !user_suspended?
end
def destroy?
create?
end
alias destroy? create?
def permitted_attributes
PERMITTED_ATTRIBUTES

View file

@ -3,9 +3,7 @@ class GithubRepoPolicy < ApplicationPolicy
!user_suspended? && user.authenticated_through?(:github)
end
def update_or_create?
!user_suspended? && user.authenticated_through?(:github)
end
alias update_or_create? index?
def permitted_attributes
%i[github_id_code featured]

View file

@ -3,29 +3,17 @@ class HtmlVariantPolicy < ApplicationPolicy
minimal_admin?
end
def show?
minimal_admin?
end
alias show? minimal_admin?
def edit?
minimal_admin?
end
alias edit? minimal_admin?
def update?
minimal_admin?
end
alias update? minimal_admin?
def new?
minimal_admin?
end
alias new? minimal_admin?
def create?
minimal_admin?
end
alias create? minimal_admin?
def destroy?
minimal_admin?
end
alias destroy? minimal_admin?
def permitted_attributes
%i[html name published approved target_tag group]

View file

@ -3,21 +3,15 @@ class ListingPolicy < ApplicationPolicy
user_author? || authorized_organization_admin_editor?
end
def update?
user_author? || authorized_organization_admin_editor?
end
alias update? edit?
def authorized_organization_poster?
user.org_member?(record.organization_id)
end
def delete_confirm?
update?
end
alias delete_confirm? edit?
def destroy?
update?
end
alias destroy? edit?
private

View file

@ -7,9 +7,7 @@ class MessagePolicy < ApplicationPolicy
user_sender?
end
def update?
destroy?
end
alias update? destroy?
def permitted_attributes_for_update
%i[message_markdown]

View file

@ -27,7 +27,5 @@ class OrganizationPolicy < ApplicationPolicy
OrganizationMembership.exists?(user_id: user.id, organization_id: record.id, type_of_user: "admin")
end
def generate_new_secret?
update?
end
alias generate_new_secret? update?
end

View file

@ -3,11 +3,7 @@ class PinnedArticlePolicy < ApplicationPolicy
user&.any_admin?
end
def update?
show?
end
alias update? show?
def destroy?
show?
end
alias destroy? show?
end

View file

@ -13,9 +13,7 @@ class ResponseTemplatePolicy < ApplicationPolicy
user_moderator?
end
def create?
true
end
alias create? index?
# comes from comments_controller
def moderator_create?
@ -26,9 +24,7 @@ class ResponseTemplatePolicy < ApplicationPolicy
user_owner?
end
def update?
user_owner?
end
alias update? destroy?
def permitted_attributes_for_create
PERMITTED_ATTRIBUTES

View file

@ -3,9 +3,7 @@ class StripeActiveCardPolicy < ApplicationPolicy
!user_suspended?
end
def update?
!user_suspended?
end
alias update? create?
def destroy?
true

View file

@ -3,9 +3,7 @@ class StripeSubscriptionPolicy < ApplicationPolicy
!user_suspended?
end
def update?
!user_suspended?
end
alias update? create?
def destroy?
true

View file

@ -7,9 +7,7 @@ class TagPolicy < ApplicationPolicy
has_mod_permission?
end
def update?
has_mod_permission?
end
alias update? edit?
def admin?
user_admin?

View file

@ -3,9 +3,7 @@ class UserBlockPolicy < ApplicationPolicy
!user_suspended?
end
def destroy?
!user_suspended?
end
alias destroy? create?
def permitted_attributes
%i[id blocked_id]

View file

@ -54,33 +54,21 @@ class UserPolicy < ApplicationPolicy
true
end
def onboarding_checkbox_update?
true
end
alias onboarding_checkbox_update? onboarding_update?
def onboarding_notifications_checkbox_update?
true
end
alias onboarding_notifications_checkbox_update? onboarding_update?
def update?
current_user? && !user_suspended?
edit? && !user_suspended?
end
def destroy?
current_user?
end
alias destroy? edit?
def confirm_destroy?
current_user?
end
alias confirm_destroy? edit?
def full_delete?
current_user?
end
alias full_delete? edit?
def request_destroy?
current_user?
end
alias request_destroy? edit?
def join_org?
!user_suspended?
@ -90,9 +78,7 @@ class UserPolicy < ApplicationPolicy
OrganizationMembership.exists?(user_id: user.id, organization_id: record.id)
end
def remove_identity?
current_user?
end
alias remove_identity? edit?
def dashboard_show?
current_user? || user_admin? || minimal_admin?
@ -102,9 +88,7 @@ class UserPolicy < ApplicationPolicy
(user.has_trusted_role? || minimal_admin?) && !user.suspended?
end
def update_password?
current_user?
end
alias update_password? edit?
def permitted_attributes
PERMITTED_ATTRIBUTES

View file

@ -3,9 +3,7 @@ class VideoPolicy < ApplicationPolicy
user.created_at < 2.weeks.ago if user.created_at
end
def create?
user.created_at < 2.weeks.ago if user.created_at
end
alias create? new?
def enabled?
Settings::General.enable_video_upload