Merge pull request #9915 from loomio/revoke-memberships

Rename memberships.archived_at to memberships.revoked_at (and follow on changes)

app/admin/users.rb

@@ -122,7 +122,7 @@ def find_resource
     end
 
     panel("Memberships") do
-      table_for user.memberships.includes(:group, :user).order(:id).each do |m|
+      table_for user.memberships.includes(:group, :user, :revoker).order(:id).each do |m|
         column :id
         column :group_name do |g|
           group = g.group
@@ -131,7 +131,10 @@ def find_resource
         column :volume
         column :admin
         column :accepted_at
-        column :archived_at
+        column :revoked_at
+        column :revoker do |m|
+          m.revoker.present? ? m.revoker.name : ''
+        end
       end
     end
 

app/controllers/api/b3/users_controller.rb

@@ -9,8 +9,8 @@ def authenticate_api_key!
   end
 
   def deactivate
-    User.active.find(params[:id]) # throws 404 if not present
-    DeactivateUserWorker.perform_async(params[:id])
+    user = User.active.find(params[:id]) # throws 404 if not present
+    DeactivateUserWorker.perform_async(user.id, user.id)
     render json: {success: :ok}
   end
 

app/controllers/api/v1/profile_controller.rb

@@ -80,7 +80,7 @@ def avatar_uploaded
   end
 
   def destroy
-    service.deactivate(user: current_user)
+    service.deactivate(user: current_user, actor: current_user)
     respond_with_resource
   end
 

app/extras/queries/users_by_volume_query.rb

@@ -25,7 +25,7 @@ def self.users_by_volume(model, operator, volume)
       joins("LEFT OUTER JOIN discussion_readers dr ON dr.discussion_id = #{model.discussion_id || 0} AND dr.user_id = users.id").
       joins("LEFT OUTER JOIN memberships m ON m.user_id = users.id AND m.group_id = #{model.group_id || 0}").
       joins("LEFT OUTER JOIN stances s ON s.participant_id = users.id AND s.poll_id = #{model.poll_id || 0} AND s.latest = TRUE").
-      where('(m.id IS NOT NULL AND m.archived_at IS NULL) OR
+      where('(m.id IS NOT NULL AND m.revoked_at IS NULL) OR
              (dr.id IS NOT NULL and dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL) OR
              (s.id IS NOT NULL and s.revoked_at IS NULL AND s.inviter_id IS NOT NULL) OR
              (m.id IS NULL and dr.id IS NULL and s.id IS NULL)').

app/mailers/event_mailer.rb

@@ -16,9 +16,18 @@ def event(recipient_id, event_id)
       @discussion = @event.eventable.discussion
     end
 
-    if @event.kind == "membership_created" && @event.eventable_type == "Group"
-      # if the membership has been deleted, let it go
-      return unless Membership.where(user_id: recipient_id, group_id: @event.eventable_id).exists?
+    if @event.eventable.respond_to?(:group_id) && @event.eventable.group_id
+      @membership = Membership.active.find_by(
+        group_id: @event.eventable.group_id,
+        user_id: recipient_id
+      )
+
+      # this might be necessary to comply with anti-spam rules
+      # if someone does not respond to the invitation, don't send them more emails
+      return if @membership &&
+                !@recipient.email_verified &&
+                !@membership.accepted_at &&
+                !["membership_created", "membership_resent"].include?(@event.kind)
     end
 
     @utm_hash = { utm_medium: 'email', utm_campaign: @event.kind }

app/models/discussion.rb

@@ -88,7 +88,7 @@ def self.pg_search_insert_statement(id: nil, author_id: nil)
   has_many :items, -> { includes(:user) }, class_name: 'Event', dependent: :destroy
 
   has_many :discussion_readers, dependent: :destroy
-  has_many :readers,-> { merge DiscussionReader.not_revoked },  through: :discussion_readers, source: :user
+  has_many :readers,-> { merge DiscussionReader.active },  through: :discussion_readers, source: :user
   has_many :guests, -> { merge DiscussionReader.guests }, through: :discussion_readers, source: :user
   has_many :admin_guests, -> { merge DiscussionReader.admins }, through: :discussion_readers, source: :user
   include DiscussionExportRelations
@@ -145,15 +145,15 @@ def members
     User.active.
       joins("LEFT OUTER JOIN discussion_readers dr ON dr.discussion_id = #{self.id || 0} AND dr.user_id = users.id").
       joins("LEFT OUTER JOIN memberships m ON m.user_id = users.id AND m.group_id = #{self.group_id || 0}").
-      where('(m.id IS NOT NULL AND m.archived_at IS NULL) OR
+      where('(m.id IS NOT NULL AND m.revoked_at IS NULL) OR
              (dr.id IS NOT NULL and dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL)')
   end
 
   def admins
     User.active.
       joins("LEFT OUTER JOIN discussion_readers dr ON dr.discussion_id = #{self.id || 0} AND dr.user_id = users.id").
       joins("LEFT OUTER JOIN memberships m ON m.user_id = users.id AND m.group_id = #{self.group_id || 0}").
-      where('(m.admin = TRUE AND m.id IS NOT NULL AND m.archived_at IS NULL) OR
+      where('(m.admin = TRUE AND m.id IS NOT NULL AND m.revoked_at IS NULL) OR
              (dr.admin = TRUE AND dr.id IS NOT NULL and dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL)')
   end
 

app/models/discussion_reader.rb

@@ -12,9 +12,8 @@ class DiscussionReader < ApplicationRecord
   delegate :message_channel, to: :user
 
   scope :dangling, -> { joins('left join discussions on discussions.id = discussion_id left join users on users.id = user_id').where('discussions.id is null or users.id is null') }
-  # scope :spammy, -> { joins('left join users on users.id = user_id').where('users.email_verified = false') }
 
-  scope :not_revoked, -> { where("discussion_readers.revoked_at IS NULL") }
+  scope :active, -> { where("discussion_readers.revoked_at IS NULL") }
 
   scope :guests, -> { where("discussion_readers.inviter_id IS NOT NULL
                               AND discussion_readers.revoked_at IS NULL") }

app/models/group.rb

@@ -35,13 +35,13 @@ class Group < ApplicationRecord
   has_many :all_memberships, dependent: :destroy, class_name: 'Membership'
   has_many :all_members, through: :all_memberships, source: :user
 
-  has_many :memberships, -> { where archived_at: nil }
+  has_many :memberships, -> { active }
   has_many :members, through: :memberships, source: :user
 
-  has_many :accepted_memberships, -> { accepted }, class_name: 'Membership'
+  has_many :accepted_memberships, -> { active.accepted }, class_name: "Membership"
   has_many :accepted_members, through: :accepted_memberships, source: :user
 
-  has_many :admin_memberships, -> { where admin: true, archived_at: nil }, class_name: 'Membership'
+  has_many :admin_memberships, -> { active.where(admin: true) }, class_name: 'Membership'
   has_many :admins, through: :admin_memberships, source: :user
 
   has_many :membership_requests, dependent: :destroy
@@ -271,18 +271,16 @@ def ensure_handle_is_not_empty
 
   def archive!
     Group.where(id: id_and_subgroup_ids).update_all(archived_at: DateTime.now)
-    Membership.where(group_id: id_and_subgroup_ids).update_all(archived_at: DateTime.now)
     reload
   end
 
   def unarchive!
-    Group.where(id: all_subgroup_ids.concat([id])).update_all(archived_at: nil)
-    Membership.where(group_id: all_subgroup_ids.concat([id])).update_all(archived_at: nil)
+    Group.where(id: id_and_subgroup_ids).update_all(archived_at: nil)
     reload
   end
 
   def org_memberships_count
-    Membership.not_archived.where(group_id: id_and_subgroup_ids).count('distinct user_id')
+    Membership.active.where(group_id: id_and_subgroup_ids).count('distinct user_id')
   end
 
   def org_members_count
@@ -328,7 +326,7 @@ def full_name
   end
 
   def id_and_subgroup_ids
-    subgroup_ids.concat([id]).compact
+    subgroup_ids.concat([id]).compact.uniq
   end
 
   def identity_for(type)

app/models/membership.rb

@@ -23,14 +23,15 @@ def initialize(obj)
   belongs_to :group
   belongs_to :user
   belongs_to :inviter, class_name: 'User'
+  belongs_to :revoker, class_name: 'User'
   has_many :events, as: :eventable, dependent: :destroy
 
   scope :dangling,      -> { joins('left join groups g on memberships.group_id = g.id').where('group_id is not null and g.id is null')  }
-  scope :active,        -> { not_archived.accepted }
-  scope :archived,      -> { where('archived_at IS NOT NULL') }
-  scope :not_archived,  -> { where(archived_at: nil) }
-  scope :pending,       -> { where(accepted_at: nil).where("user_id is not null") }
+  scope :user_active,   -> { joins(:user).where("users.deactivated_at is null") }
+  scope :active,        -> { where(revoked_at: nil) }
+  scope :pending,       -> { active.where(accepted_at: nil) }
   scope :accepted,      -> { where('accepted_at IS NOT NULL') }
+  scope :revoked,       -> { where('revoked_at IS NOT NULL') }
 
   scope :search_for, ->(query) { joins(:user).where("users.name ilike :query or users.username ilike :query or users.email ilike :query", query: "%#{query}%") }
 
@@ -39,7 +40,7 @@ def initialize(obj)
   scope :for_group, lambda {|group| where(group_id: group)}
   scope :admin, -> { where(admin: true) }
 
-  has_paper_trail only: [:group_id, :user_id, :inviter_id, :admin, :title, :archived_at, :volume, :accepted_at]
+  has_paper_trail only: [:group_id, :user_id, :inviter_id, :admin, :title, :revoked_at, :revoker_id, :volume, :accepted_at]
   delegate :name, :email, to: :user, prefix: :user, allow_nil: true
   delegate :parent, to: :group, prefix: :group, allow_nil: true
   delegate :name, :full_name, to: :group, prefix: :group

app/models/poll.rb

@@ -393,7 +393,7 @@ def admins
              (p.author_id = users.id AND p.group_id IS NULL) OR
              (p.author_id = users.id AND dr.id IS NOT NULL AND dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL) OR
              (dr.id IS NOT NULL AND dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL AND dr.admin = TRUE) OR
-             (m.id  IS NOT NULL AND m.archived_at IS NULL AND m.admin = TRUE) OR
+             (m.id  IS NOT NULL AND m.revoked_at IS NULL AND m.admin = TRUE) OR
              (s.id  IS NOT NULL AND s.revoked_at  IS NULL AND latest = TRUE AND s.admin = TRUE)")
   end
 
@@ -404,7 +404,7 @@ def members
       joins("LEFT OUTER JOIN memberships m ON m.user_id = users.id AND m.group_id = #{self.group_id || 0}").
       joins("LEFT OUTER JOIN stances s ON s.participant_id = users.id AND s.poll_id = #{self.id || 0}").
       where("(dr.id IS NOT NULL AND dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL) OR
-             (m.id  IS NOT NULL AND m.archived_at IS NULL) OR
+             (m.id  IS NOT NULL AND m.revoked_at IS NULL) OR
              (s.id  IS NOT NULL AND s.revoked_at  IS NULL AND latest = TRUE)")
   end
 

app/models/user.rb

@@ -18,7 +18,7 @@ class User < ApplicationRecord
   extend NoSpam
   no_spam_for :name, :email
 
-  has_paper_trail only: [:email_newsletter]
+  has_paper_trail only: [:email_newsletter, :deactivated_at, :deactivator_id]
 
   MAX_AVATAR_IMAGE_SIZE_CONST = 100.megabytes
   BOT_EMAILS = {
@@ -67,17 +67,9 @@ class User < ApplicationRecord
            -> { where('memberships.admin = ?', true) },
            class_name: 'Membership'
 
-  has_many :memberships, -> { where(archived_at: nil) }, dependent: :destroy
+  has_many :memberships, -> { active }, dependent: :destroy
   has_many :all_memberships, dependent: :destroy, class_name: "Membership"
 
-  has_many :archived_memberships,
-           -> { where('archived_at IS NOT NULL') },
-           class_name: 'Membership'
-
-  has_many :invited_memberships,
-           class_name: 'Membership',
-           foreign_key: :inviter_id
-
   has_many :groups,
            through: :memberships,
            class_name: 'Group',
@@ -144,7 +136,7 @@ class User < ApplicationRecord
     active.verified.search_for(query).
       joins("LEFT OUTER JOIN memberships m ON m.user_id = users.id AND m.group_id = #{model.group_id || 0}").
       joins("LEFT OUTER JOIN discussion_readers dr ON dr.user_id = users.id AND dr.discussion_id = #{model.discussion_id || 0}").
-      where("(m.id IS NOT NULL AND m.archived_at IS NULL) OR
+      where("(m.id IS NOT NULL AND m.revoked_at IS NULL) OR
              (dr.id IS NOT NULL AND dr.inviter_id IS NOT NULL AND dr.revoked_at IS NULL)")
   end
 

app/queries/poll_query.rb

@@ -23,7 +23,7 @@ def self.visible_to(user: LoggedOutUser.new,
                  .joins("LEFT OUTER JOIN stances s ON s.poll_id = polls.id AND (s.participant_id = #{user.id || 0} #{or_stance_token})")
                  .where("#{'d.private = false OR ' if show_public}
                          polls.author_id = :user_id OR
-                         (m.id IS NOT NULL AND m.archived_at IS NULL) OR
+                         (m.id IS NOT NULL AND m.revoked_at IS NULL) OR
                          (dr.id IS NOT NULL AND dr.revoked_at IS NULL AND dr.inviter_id IS NOT NULL) OR
                          (s.id IS NOT NULL AND s.revoked_at IS NULL)", user_id: user.id)
     chain

app/queries/user_query.rb

@@ -15,7 +15,7 @@ def self.relations(model:, actor:)
       end
 
       rels.push User.joins('LEFT OUTER JOIN memberships m ON m.user_id = users.id').
-                     where('(m.group_id IN (:group_ids))', {group_ids: group_ids})
+                     where('m.group_id IN (:group_ids) AND m.revoked_at IS NULL', {group_ids: group_ids})
 
       # people who have requested membership
       rels.push User.joins('LEFT OUTER JOIN membership_requests mr ON mr.requestor_id = users.id').
@@ -55,19 +55,19 @@ def self.relations(model:, actor:)
       if model.discussion_id
         rels.push(
           User.joins('LEFT OUTER JOIN discussion_readers dr ON dr.user_id = users.id').
-          where('dr.discussion_id': model.discussion_id)
+          where('dr.discussion_id': model.discussion_id).where('dr.revoked_at IS NULL')
         )
 
         rels.push(
           User.joins('LEFT OUTER JOIN stances ON stances.participant_id = users.id').
-          where('stances.poll_id': model.discussion.poll_ids)
+          where('stances.poll_id': model.discussion.poll_ids).where("stances.revoked_at IS NULL")
         )
       end
 
       if model.poll_id
         rels.push(
           User.joins('LEFT OUTER JOIN stances ON stances.participant_id = users.id').
-          where('stances.poll_id': model.poll_id)
+          where('stances.poll_id': model.poll_id).where("stances.revoked_at IS NULL")
         )
       end
     end

app/services/announcement_service.rb

@@ -7,8 +7,8 @@ def self.audience_users(model, kind, actor, exclude_members = false, include_act
       id = kind.match(/group-(\d+)/)[1].to_i
       group = model.group.parent_or_self.self_and_subgroups.find(id)
       raise CanCan::AccessDenied unless actor.can?(:notify, group)
-      group.accepted_members
-    when 'group'            then model.group.accepted_members
+      group.members
+    when 'group'            then model.group.members
     when 'discussion_group' then (model.discussion || NullDiscussion.new).readers
     when 'voters'           then (model.poll || NullPoll.new).unmasked_voters
     when 'decided_voters'   then (model.poll || NullPoll.new).unmasked_decided_voters

app/services/group_service.rb

@@ -62,8 +62,7 @@ def self.invite(group:, params:, actor:)
       recipient_message: params[:recipient_message])
 
     # EventBus.broadcast('group_invite', group, actor, all_memberships.size)
-    Membership.not_archived.where(group_id: group.id, user_id: users.pluck(:id))
-
+    Membership.active.where(group_id: group.id, user_id: users.pluck(:id))
   end
 
   def self.create(group:, actor: )