FEATURE: limits who can create/act on events based on groups

FEATURE: limits who can create/act on events based on groups

diff --git a/app/controllers/discourse_post_event/events_controller.rb b/app/controllers/discourse_post_event/events_controller.rb
index fd6ef0c..7c080d9 100644
--- a/app/controllers/discourse_post_event/events_controller.rb
+++ b/app/controllers/discourse_post_event/events_controller.rb
@@ -64,7 +64,7 @@ module DiscoursePostEvent
     def create
       event = Event.new(event_params)
       guardian.ensure_can_edit!(event.post)
-      guardian.ensure_can_create_event!(event)
+      guardian.ensure_can_create_event!
       event.enforce_utc!(event_params)
 
       case event_params[:status].to_i
diff --git a/app/models/discourse_post_event/event.rb b/app/models/discourse_post_event/event.rb
index eee307c..543d89f 100644
--- a/app/models/discourse_post_event/event.rb
+++ b/app/models/discourse_post_event/event.rb
@@ -10,7 +10,7 @@ module DiscoursePostEvent
 
     after_commit :destroy_topic_custom_field, on: [:destroy]
     def destroy_topic_custom_field
-      if self.post.is_first_post?
+      if self.post && self.post.is_first_post?
         TopicCustomField
           .where(
             topic_id: self.post.topic_id,
@@ -22,7 +22,7 @@ module DiscoursePostEvent
 
     after_commit :upsert_topic_custom_field, on: [:create, :update]
     def upsert_topic_custom_field
-      if self.post.is_first_post?
+      if self.post && self.post.is_first_post?
         TopicCustomField
           .upsert({
             topic_id: self.post.topic_id,
diff --git a/app/models/guardian.rb b/app/models/guardian.rb
deleted file mode 100644
index 2251d35..0000000
--- a/app/models/guardian.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-class ::Guardian
-  module CanActOnEvent
-    def can_act_on_event?(event)
-      @user.staff? || @user.admin? || @user.id == event.post.user_id
-    end
-  end
-  prepend CanActOnEvent
-
-  module CanActOnInvitee
-    def can_act_on_invitee?(invitee)
-      @user.staff? || @user.admin? || @user.id == invitee.user_id
-    end
-  end
-  prepend CanActOnInvitee
-
-  module CanCreateEvent
-    def can_create_event?(event)
-      @user.staff? || @user.admin?
-    end
-  end
-  prepend CanCreateEvent
-
-  module CanJoinEvent
-    def can_join_post_event?(event)
-      event.status === DiscoursePostEvent::Event.statuses[:public] || (
-        event.status === DiscoursePostEvent::Event.statuses[:private]
-        event.invitees.find_by(user_id: @user.id)
-      )
-    end
-  end
-  prepend CanJoinEvent
-end
diff --git a/assets/javascripts/initializers/add-event-ui-builder.js.es6 b/assets/javascripts/initializers/add-event-ui-builder.js.es6
index 8c28c90..eb5d336 100644
--- a/assets/javascripts/initializers/add-event-ui-builder.js.es6
+++ b/assets/javascripts/initializers/add-event-ui-builder.js.es6
@@ -3,10 +3,9 @@ import showModal from "discourse/lib/show-modal";
 
 function initializeEventBuilder(api) {
   const currentUser = api.getCurrentUser();
-  const siteSettings = api.container.lookup("site-settings:main");
 
   api.onToolbarCreate(toolbar => {
-    if (!currentUser.staff) {
+    if (!currentUser || !currentUser.can_create_event) {
       return;
     }
 
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 648eea4..14c64c3 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -30,5 +30,7 @@ en:
           ends_at_before_starts_at: "An event can't end before it starts."
           start_must_be_present_and_a_valid_date: "An event requires a valid start date."
           end_must_be_a_valid_date: "End date must be a valid date."
+          acting_user_not_allowed_to_create_event: "Current user is not allowed to create events."
+          acting_user_not_allowed_to_act_on_this_event: "Current user is not allowed to act on this event."
           name:
             length: "Event name length must be between %{minimum} and %{maximum} characters."
diff --git a/config/settings.yml b/config/settings.yml
index 35eed1b..09a3781 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -1,7 +1,8 @@
-discourse_calendar:
+plugins:
   calendar_enabled:
     default: false
     client: true
+discourse_calendar:
   holiday_calendar_topic_id:
     default: ""
     client: true
@@ -45,6 +46,13 @@ discourse_post_event:
   discourse_post_event_enabled:
     default: false
     client: true
+  discourse_post_event_allowed_on_groups:
+    client: true
+    type: group_list
+    list_type: compact
+    default: ""
+    allow_any: false
+    refresh: true
   displayed_invitees_limit:
     default: 10
     client: false
diff --git a/lib/discourse_post_event/event_validator.rb b/lib/discourse_post_event/event_validator.rb
index 192da94..d608559 100644
--- a/lib/discourse_post_event/event_validator.rb
+++ b/lib/discourse_post_event/event_validator.rb
@@ -25,6 +25,18 @@ module DiscoursePostEvent
 
       extracted_event = extracted_events.first
 
+      if @post.acting_user && @post.event
+        if !@post.acting_user.can_act_on_event?(@post.event)
+          @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.acting_user_not_allowed_to_act_on_this_event"))
+          return false
+        end
+      else
+        if !@post.acting_user || !@post.acting_user.can_create_event?
+          @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.acting_user_not_allowed_to_create_event"))
+          return false
+        end
+      end
+
       if extracted_event[:start].blank? || (DateTime.parse(extracted_event[:start]) rescue nil).nil?
         @post.errors.add(:base, I18n.t("discourse_post_event.errors.models.event.start_must_be_present_and_a_valid_date"))
         return false
diff --git a/plugin.rb b/plugin.rb
index 8a94bb1..d5d1f99 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -125,6 +125,42 @@ after_initialize do
     end
   end
 
+  add_to_class(:user, :can_create_event?) do
+    @can_create_event ||=
+      begin
+        return true if admin? || staff?
+        allowed_groups = SiteSetting.discourse_post_event_allowed_on_groups.split('|').compact
+        allowed_groups.present? && groups.where(id: allowed_groups).exists? ?
+          :true : :false
+      end
+    @can_create_event == :true
+  end
+
+  add_to_class(:guardian, :can_act_on_invitee?) do |invitee|
+    user && (user.staff? || user.admin? || user.id == invitee.user_id)
+  end
+
+  add_to_class(:guardian, :can_create_event?) { user && user.can_create_event? }
+
+  add_to_serializer(:current_user, :can_create_event) do
+    object.can_create_event?
+  end
+
+  add_to_class(:user, :can_act_on_event?) do |event|
+    @can_act_on_event ||=
+      begin
+        return true if admin? || staff?
+        can_create_event? && event.post.user_id == id ? :true : :false
+      end
+    @can_act_on_event == :true
+  end
+
+  add_to_class(:guardian, :can_act_on_event?) { |event| user && user.can_act_on_event?(event) }
+
+  add_class_method(:group, :discourse_post_event_allowed_groups) do
+    where(id: SiteSetting.discourse_post_event_allowed_on_groups.split('|').compact)
+  end
+
   add_to_serializer(:post, :event) do
     DiscoursePostEvent::EventSerializer.new(object.event, scope: scope, root: false)
   end
@@ -232,7 +268,6 @@ after_initialize do
 
   [
     "../app/models/calendar_event.rb",
-    "../app/models/guardian.rb",
     "../app/serializers/user_timezone_serializer.rb",
     "../jobs/scheduled/create_holiday_events.rb",
     "../jobs/scheduled/destroy_past_events.rb",
diff --git a/spec/acceptance/post_spec.rb b/spec/acceptance/post_spec.rb
index 0248c4e..53a6242 100644
--- a/spec/acceptance/post_spec.rb
+++ b/spec/acceptance/post_spec.rb
@@ -6,17 +6,18 @@ require_relative '../fabricators/event_fabricator'
 describe Post do
   Event ||= DiscoursePostEvent::Event
 
-  fab!(:user) { Fabricate(:user) }
-  fab!(:topic) { Fabricate(:topic, user: user) }
-  fab!(:post1) { Fabricate(:post, topic: topic) }
-  fab!(:post_event) { Fabricate(:event, post: post1) }
-
   before do
     freeze_time

[... diff too long, it was truncated ...]

GitHub sha: cde452ca

staff? covers the admin? case too

1 Like

I really would like us to move away from this pattern of using symbols for true and false - can we change this to use defined? instead for memoization of booleans?

https://jtway.co/how-to-memoize-false-and-nil-values-fe28b8ede9f8

1 Like

Same feedback here (re: symbols)

1 Like

Fixed in FIX: prevents name collision by namespacing guardian method · discourse/discourse-calendar@cc193bf · GitHub