Updating a record fails if a column has original value of false

[akadusei]

The following fails, with an error:

SaveUserOptions.update(
  user.options!, # user.options! has `password_notify` = false, `login_notify` = true
  params(login_notify: false) # `password_notify` is not updated here
) do |operation, updated_user_options|
  # ...
end

The error is {:password_notify => ["is required"]}

Seems the operation fails because password_notify has original value of false. If you explicitly pass password_notify: false to the operation, it works OK. Otherwise, if password_notify has original value of true, it works OK.

This seems to be a regression in #424. My guess is this line:

avram/src/avram/save_operation.cr

Line 126 in fedd1be

value: @record.try(&.{{ attribute[:name] }}) || default_value_for_{{ attribute[:name] }},

[jwoertink]

Thanks for the report. I'll take a look at this.

[akadusei]

This change seems to fix it:

# ...
  private def _{{ attribute[:name] }}
    record_value = @record.try(&.{{ attribute[:name] }})
    value = record_value.nil? ? default_value_for_{{ attribute[:name] }} : record_value
    
    @_{{ attribute[:name] }} ||= Avram::Attribute({{ attribute[:type] }}?).new(
       name: :{{ attribute[:name].id }},
       param: permitted_params["{{ attribute[:name] }}"]?,
       value: value,
       param_key: self.class.param_key)
  end
#...

[jwoertink]

Ok, I'm taking a look in to this, but I'm having some trouble understanding what the issue is. Using Avram master, I wrote these two specs:

model = ModelWithDefaultValuesBox.create
params = Avram::Params.new({"greeting" => "Hi"})
OverrideDefaults.update(model, params) do |_operation, record|
  record.should_not eq nil
  r = record.not_nil!
  r.greeting.should eq "Hi"
  r.admin.should eq false
end


user = UserBox.create &.available_for_hire(true)
params = Avram::Params.new({"available_for_hire" => "false"})
SaveUser.update(user, params) do |_operation, record|
  record.should_not eq nil
  r = record.not_nil!
  r.available_for_hire.should eq false
end

and both of these specs pass. In this case, the ModelWithDefaultValues has a admin = false default, and User has available_for_hire is nilable, but set to true by default. I'm able to update it to false through params, and it updates.

Can you give a little more context as to what user.options! is? Does this return a model? Is that field permitted? Or are you able to share any more code?

[akadusei]

Can you give a little more context as to what user.options! is? Does this return a model? Is that field permitted? Or are you able to share any more code?

user.options! returns a UserOptions model, defined as follows:

class UserOptions < BaseModel
  table :user_options do
    belongs_to user : User

    column login_notify : Bool
    column password_notify : Bool
  end
end

The two columns have no default values.

In SaveUserOptions, both columns are permitted:

class SaveUserOptions < UserOptions::SaveOperation
  permit_columns :login_notify, :password_notify

  before_save do
    validate_required login_notify, password_notify
    validate_required user_id, message: "does not exist"
  end
end

If user.options! has login_notify = true and password_notify = true, the following works:

SaveUserOptions.update(
  user.options!,
  # NOTICE we are not passing `password_notify` here, even though it is required.
  # I don't know if this was intended, but that is how Avram works currently.
  # It seems Avram merges in the columns from the record (`user.options!`)
  # in update operations.
  Avram::Params.new({"login_notify" => "false"})
) do |operation, updated_user_options|
  updated_user_options.login_notify.should be_false # <= PASSES
  updated_user_options.password_notify.should be_true # <= PASSES
end

However, If user.options! has login_notify = true and password_notify = false, the same operation fails, with {:password_notify => ["is required"]}.

Avram requires the password_notify param to be passed in explicitly, in the SaveUserOptions.update call, only when it is originally false.

Hope this helps?

[akadusei]

avram/src/avram/save_operation.cr

Line 126 in fedd1be

value: @record.try(&.{{ attribute[:name] }}) || default_value_for_{{ attribute[:name] }},

I believe this bug happened in the line of code above, when it was changed from value: @record.try(&.{{ attribute[:name] }}), to value: @record.try(&.{{ attribute[:name] }}) || default_value_for_{{ attribute[:name] }},.

The added || default_value_for_{{ attribute[:name] }}, makes record columns with false values fall through to the default_value_for_{{ attribute[:name] }} method, which in my case returns nil because it has no default value. Hence, the error.

[jwoertink]

Ah, ok. Yeah, that helps. I have a failing spec now which seems to be related to the validate_required. Removing the validate_required password_notify lets it pass. Your fix may be the solution, but I want to double check that this isn't actually a bug in the validation. Thanks for the clarification!