Maxence
Maxence

Reputation: 2339

Destroy record instead of update in form nested model

I have a model opening_times that records opening times of shops.

create_table "opening_times", force: :cascade do |t|
    t.string "day"
    t.time "morning"
    t.time "evening"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.bigint "shop_id"
    t.index ["shop_id"]
  end

This model is updated only through a nested form in the edit action of shops controller. I have no opening_times controller.

Then basically what I can do is pretty much limited: the update action.

Though I have a problem: when there used to be opening times for a specific day, let's say tuesday, and the user want to make that day not worked, the user makes both fields morning and evening blanks.

I could save empty values in the database but it would be better to actually delete the record for tuesday.

then in the model file I have set this :

before_save :delete_records_with_missing_hours

private 

def delete_records_with_missing_hours
    if self.morning.blank? or self.evening.blank?
        self.destroy
    end
end

But it doesn't work.

Is there a way to delete a record that was intended to be updated at the model level ?

Upvotes: 2

Views: 1673

Answers (3)

ogabriel
ogabriel

Reputation: 301

If you rather not have fields with empty values for morning and evening, why not just validate the existence of those fields? It would be much more cleaner then using external methods to validate that situation.

Firstly, you should put this in the Shop model:

validates_associated :opening_times

This code will make sure that your association is validated before being inserted/updated

Now you can put a validation in the OpeningTime model, like this:

validates :morning, :evening, presence: true

And if you just wanna do that in the update action, you can even do:

validates :morning, :evening, presence: true, on: :update

And if you wanted at least one of the values to be present, you can also use:

validate :morning_and_evening_validation

def morning_and_evening_validation
  morning.present? || evening.present?
end

I think mostly it's much more cleaner and readable.

Upvotes: 0

moveson
moveson

Reputation: 5213

There is a Rails Way to handle this. It can be done very easily, but you need to do it from your Shop model. In that model insert this:

class Shop < ApplicationRecord
  accepts_nested_attributes_for :opening_times, allow_destroy: true, reject_if: :reject_opening_time?

  def reject_opening_time?(attributes)
    persisted = attributes[:id].present?
    time_values = attributes.slice(:morning, :evening).values
    without_time = time_values.any?(&:blank?)
    attributes.merge!(_destroy: true) if persisted and without_time
    without_time && !persisted # Return false so as to reject new opening_time if any time attributes are empty
  end
end

Now for each opening_time nested record, Rails will evaluate the time attributes. If any time value is blank, it will treat the record appropriately. If the record is persisted, it will add a _destroy attribute, which will destroy the nested record when you save the parent. If the record is not persisted, it will be rejected (ignored) when you save the parent.

Upvotes: 1

bo-oz
bo-oz

Reputation: 2872

I think you are better of creating some kind of maintenance script for this action. Like a rake task or something. So periodically you run a simple find and destroy:

OpeningTime.where(morning:nil, evening:nil).destroy_all

Just run this on a daily or weekly basis to clean up your database and don't bother with the records at the moment of saving.

I think the problem with your current code is that you are deleting the record in the before_save, so this creates two problems:

1/ What should rails do if this is not a persisted record, but a new one? Destroy would fail and with it the entire transaction will rollback (take a look at your console)

2/ Even if the record was persisted, deleting it would then work, but the save action that will be performed next would fail, also resulting in a rollback.

Upvotes: 1

Related Questions