mercredi 25 mai 2016

Using an if statement within a transaction to delete multiple objects

I'm a junior dev trying to write code that separates out beeps that are owned by a user and beeps that another user is authorized for.

The beeps come from a device that a user owns. An owner can authorize other users to use that device and receive their own beep alerts. This gives us two different types of beeps: owned beeps and authorized beeps. I want the authorized user to be able to delete multiple beep events at the same time and only their own, while I want the owner to be able to delete multiple beeps and those beep's corresponding events.

The application I'm working on is very large and architecturally has been set up with this functionality for the owner to delete only single beeps.

The beep events go through a method that talks to a separate application that handles the deletion of events through the device_broker method.

What I ended up doing was a transaction with an if statement and then looping through an each iterator, checking each beep that's passing through if it was owned or authorized by the user.

My problem is that my broker_mock test isn't receiving any arguments. When I use a debugger on the code, the methods all seem to be working, so I'm very puzzled. Is using an if statement with an each loop a good idea? Also wrapping it all in a transaction means that if one tiny thing breaks, it doesn't work. Is there a better alternative?

Thanks for any wisdom or light you can shed.

  beeps_controller.rb

  def bulk_deletion
      BulkBeepRemover.run!(params[:beep_ids], current_user)
  end

  bulk_beep_remover.rb

  class BulkBeepRemover
    class Unauthorized < StandardError; end;
    attr_reader :beep_ids, :user, :beeps

    attr_accessor :owned_beeps, :unowned_beeps

    def self.run!(beep_ids, user)
      new(beep_ids, user).run!
    end

    def initialize(beep_ids, user)
      @beep_ids = beep_ids
      raise Unauthorized if user.nil?
      @user = user
    end

    def beeps
      @beeps ||= Beep.finished.find(beep_ids)
    end

    def device_broker
      @device_broker ||= $device_broker
    end

    attr_writer :device_broker

    def run!
      #handles deletion of the owned beeps
      Beep.transaction do
        beeps.each do |beep|
          if beep.device.is_owner?(user)
             beep.destroy
             remove_beep_event(beep.id)
          end
         # elsif beep.device.is_authorized?(user) logic goes here
        end
      end
    end

 private

  def remove_beep_event(beep_id)
    device_broker.publish('beep_deleted', { beep_id: beep_id })
  end

dings_spec.rb

  describe "POST /clients_api/beeps/bulk_deletion" do
    let(:user_id)        { user.id }

    let!(:shared_beep_1) { create(:beep, id: 33, device:       authorized_device, state: :completed) }
    let!(:shared_beep_2) { create(:beep, id: 34, device: authorized_device, state: :completed) }
    let!(:owned_beep_1)  { create(:beep, id: 35, device: owned_device, state: :completed) }
    let!(:owned_beep_2)  { create(:beep, id: 36, device: owned_device, state: :completed) }

    before do
      post "/clients_api/beeps/:beep_id/bulk_deletion",        default_params.merge(beep_ids: [shared_beep_1.id, shared_beep_2.id,     owned_beep_1.id, owned_beep_2.id], user_id: user.id)
      allow_any_instance_of(BulkBeepRemover).to receive(:device_broker).and_return(broker_mock)
    end

    context "the owner's array of beep ids" do
      let(:beep_id) { owned_beep_2.id }


      it "deletes the beeps" do
        expect { Beep.find(beep_id) }.to     raise_error(ActiveRecord::RecordNotFound)
      end

      it "publishes the owner beep_deleted event" do
        expect(broker_mock).to     have_received(:publish).with("beep_deleted", { beep_id: beep_id })
      end
    end
  end
end

Aucun commentaire:

Enregistrer un commentaire