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

when a record is destroyed make sure all "reactivity" is turned off during the update #195

Closed
catmando opened this issue Jun 12, 2019 · 1 comment
Labels
bug Something isn't working ready-to-release Internal Use Only: Has been fixed, specs passing and pushed to edge branch
Milestone

Comments

@catmando
Copy link
Contributor

For example this spec will fail:

  context "updating a has_many relationship" do

    before(:each) do

      m = FactoryBot.create(:test_model, test_attribute: 'hello')
      FactoryBot.create(:child_model, test_model: m)

      mount "TestComponent3" do
        class InnerComponent < HyperComponent
          param :child
          render { LI { "child id = #{@Child.id} #{@Child.test_model.test_attribute}"} }
        end
        class TestComponent3 < HyperComponent
          render(OL) do
            TestModel.all[0].child_models.each do |child|
              InnerComponent(child: child)
            end
          end
        end
      end
      page.should have_content('child id = 1')
    end

    it "will update when sent from the server" do
      ChildModel.create(child_attribute: :foo, test_model: TestModel.find(1))
      page.should have_content('child id = 2')
      ChildModel.find(1).destroy
      sleep 0.1 # necessary for poltergeist to work with pusher faker
      page.should_not have_content('child id = 1', wait: 2)
      page.should have_content('child id = 2')
    end
end

because InnerComponent accesses the child's test_model, which is set to nil when the child is destroyed (because we want to keep all the linkages nicely synchronized) but setting this to nil causes the component to re-rerender.

Here is a patch (not fully tested)

module ReactiveRecord
  class Base
    alias destroy_associations_before_patch destroy_associations
    def destroy_associations(*args, &block)
      @being_destroyed = true
      destroy_associations_before_patch
    end

    alias change_status_and_notify_helper_before_patch change_status_and_notify_helper
    def change_status_and_notify_helper(*args, &block)
      return if @being_destroyed
      change_status_and_notify_helper_before_patch(*args, &block)
    end
  end
end

Note that simply wrapping the destroy_associations method with the Base.loading method will not work, as this will turn off notification for both sides of the relationship. I.e. the has_many side will never re-render either, which it should.

@catmando
Copy link
Contributor Author

FYI above patch is equivilent to the tested code.

@catmando catmando added the bug Something isn't working label Jun 13, 2019
@catmando catmando added this to the alpha1.5 milestone Jun 13, 2019
@catmando catmando added the ready-to-release Internal Use Only: Has been fixed, specs passing and pushed to edge branch label Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-to-release Internal Use Only: Has been fixed, specs passing and pushed to edge branch
Projects
None yet
Development

No branches or pull requests

1 participant