Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yield should test if there's a block to yield to in persistence/update #160

Open
lisad opened this issue Oct 8, 2013 · 1 comment
Open

Comments

@lisad
Copy link
Contributor

lisad commented Oct 8, 2013

In lib / dynamoid / persistence.rb, line 185, there's a "yield t". There's no check to see if there's a block to yield to. If there's no block passed in, this fails with

LocalJumpError: no block given (yield)

    def update!(conditions = {}, &block)
      run_callbacks(:update) do
        options = range_key ? {:range_key => dump_field(self.read_attribute(range_key), self.class.attributes[range_key])} : {}
        new_attrs = Dynamoid::Adapter.update_item(self.class.table_name, self.hash_key, options.merge(:conditions => conditions)) do |t|
          if(self.class.attributes[:lock_version])
            raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning)
            t.add(lock_version: 1)
          end

          yield t
        end
        load(new_attrs)
      end
    end

The way this code got run as "@event.update(event_params)" without a block was in some auto-generated Rails code in my project's EventController. Here's the controller method that was autogenerated.

  def update
    respond_to do |format|
      if @event.update(event_params)
        format.html { redirect_to @event, notice: 'Event was successfully updated.' }
        format.json { head :no_content }
      else
        format.html { render action: 'edit' }
        format.json { render json: @event.errors, status: :unprocessable_entity }
      end
    end
  end

It's easy enough for me to do update differently, I don't want to do it the boilerplate way anyway. However, I thought I'd point out what's going on here, and that others might run into the same issue pretty fast if they're following Rails standard practices.

@tbtalbottjr
Copy link

I'm assuming the #update in Dynamoid is a different beast than the #update in Rails 4.0.x ActiveRecord and it is a name collision. Dynamoid still expects #update_attributes.

This should be corrected for better Rails 4.0.x support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants