CODEWITHSUNDEEP
ruby-on-railsruby
user11027209
user11027209

Reputation:

flexible system to destroy each records in batch

client_skipped_day_controller.rb

class ClientSkippedDaysController < ApplicationController
  before_action :check_client_on_exist, only: [:create]

  def index
    @client_skipped_days = ClientSkippedDay.order_by(params[:sort_by], params[:direction])

    if params[:date].present?
      @client_skipped_days = @client_skipped_days.where('skipped_at = ?', Date.parse(params[:date]))
    end

    render json: @client_skipped_days, status: :ok
  end

  def create
    @client_skipped_days = ClientSkippedDay.create!(client_skipped_days_params)

    render json: @client_skipped_days, status: :created
  end

  def destroy

  end

  private

  def client_skipped_days_params
    params.permit(client_skipped_days: %i[client_id skipped_at])[:client_skipped_days]
  end

  def check_client_on_exist
    client_skipped_days_params.each do |day|
      ClientSkippedDay.find_by(day)&.destroy
    end
  end
end

My code works if I try to delete only one record, like a :

Parameters: {"client_skipped_days"=>[{"client_id"=>533, "skipped_at"=>"2019-02-24"}], "client_skipped_day"=>{}}

But if I try to delete each hash in the array, it's didn't work :(

Parameters: {"client_skipped_days"=>[{"client_id"=>533, "skipped_at"=>"2019-02-24"}, {"client_id"=>512, "skipped_at"=>"2019-02-24"}], "client_skipped_day"=>{}}

Only one record will be deleted, but how to add the ability to delete all records? which coincide with the parameters that come from the controller?

And it must be a flexible system to remove if 1 hash in the array and immediately a collection of hashes in the array. Tell me how to do it.

Upvotes: 0

Views: 271

Answers (2)

3limin4t0r
3limin4t0r

Reputation: 21130

Instead of looping over the params and finding each record one by one you could also consider using multiple #where queries combining them together with the use of #or and loop over the resulting records.

def client_skipped_days_params
  params.permit(client_skipped_days: [:client_id, :skipped_at])
  #                                           removed `.values` ^
end

def check_client_on_exist
  destroyed_records, undestroyed_records =
    client_skipped_days_params
    .fetch(:client_skipped_days, []) # get the array or use an empty array as default
    .map(&ClientSkippedDay.method(:where)) # build individual queries
    .reduce(ClientSkippedDay.none, :or) # stitch the queries together using #or
    .partition(&:destroy) # call #destroy on each item in the collection, separating destroyed once from undestroyed once
end

In the above example the resulting destroyed records are present in the destroyed_records variable and the records that could not be destroyed are present in the undestroyed_records variable. If you don't care about the result you can leave this out. If you want to raise an exception if a record cannot be destroyed use #destroy! instead (call upon each collection item).

Alternatively you can destroy all records by calling #destroy_all (called upon the collection), but it will simply return an array of records without differentiating the destroyed records from the undestroyed records. This method will still instantiate the records and destroy them one by one with the advantage that callbacks are still triggered.

The faster option is calling #delete_all (called upon the collection). This will destroy all records with one single query. However records are not instantiated when destroyed, meaning that callbacks will not be triggered.

def check_client_on_exist
  destroyed_record_count = 
    # ...
    .reduce(ClientSkippedDay.none, :or)
    .delete_all # delete all records with a single query (without instantiation)
end

references:

Upvotes: 1

Nate
Nate

Reputation: 2404

You need to loop over your array instead of just taking the first value out of it. I don’t understand the params that you have, so I’m assuming that you want to do your find_by using the Hash of client_id and skipped_at.

Also, Ruby 2.3.0 introduced the safe navigation operator, which is what that &. is if you aren’t used to it. http://mitrev.net/ruby/2015/11/13/the-operator-in-ruby/

Since find_by either returns an ActiveRecord object or nil, it’s a great time to use the safe navigation operator to shorten things up. 

  def client_skipped_days_params
    params.permit(client_skipped_days: %i[client_id skipped_at])[:client_skipped_days]
  end

  def check_client_on_exist
    client_skipped_days_params.each do |day|
      ClientSkippedDay.find_by(day)&.destroy
    end
  end

Note, I’m not sure what your client_skipped_day Hash is. I assumed you’re making it possible to delete a single day, or delete in bulk. I would warn against having it do two things. Just make the client always send an array for this action and things will be easier for you. If you can do that, then you can make client_skipped_days required. 

  def client_skipped_days_params
    params.require(:client_skipped_days).permit(%i[client_id skipped_at])
  end

This will raise a 422 error to the client if they don’t provide the client_skipped_days key.

If this isn’t possible, then you’ll need to add an if to check_on_exist to make sure that client_skipped_days_params is not null (because they’re using client_skipped_day).

Upvotes: 1

Related Questions