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

client scope optimization doesn't work if all scope is dummy or unknown #78

Closed
catmando opened this issue Dec 6, 2018 · 4 comments
Closed
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

catmando commented Dec 6, 2018

try the todo example, with prerendering off and then in Todo model:

class Todo < ApplicationRecord
  scope :completed, -> () { where(completed: true)  }, client: -> { completed }
  scope :active,    -> () { where(completed: false) }, client: -> { !completed }
end

Load the all scope, notice that the active todo count sticks at one,
then when we look at the active scope, it only contains one (wrong usually) item.

Problem is occuring because on the footer we do a count of the active scope. Presumably this causes the completed client side scoping rule to be run, but on the dummy 'all' scope.

If you remove the footer the problem goes away.

@catmando catmando added the bug Something isn't working label Dec 6, 2018
@catmando catmando added this to the alpha1.3 milestone Dec 7, 2018
@catmando
Copy link
Contributor Author

catmando commented Dec 7, 2018

here is a failing test spec

# hyper-model/spec/batch4/scope_spec.rb:315
  it 'the client filter will not be applied to unloaded scope' do

    isomorphic do
      TestModel.class_eval do
        scope :quicker, -> { where(completed: true) }, client: -> { completed }
      end
    end

    FactoryBot.create(:test_model, test_attribute: 'model-1', completed: false)
    FactoryBot.create(:test_model, test_attribute: 'model-2', completed: true)
    FactoryBot.create(:test_model, test_attribute: 'model-3', completed: false)
    FactoryBot.create(:test_model, test_attribute: 'model-4', completed: true)

    mount 'TestComponent2' do
      class TestComponent2 < HyperComponent
        class << self
          state_accessor :scope
        end
        before_mount do
          @render_count = 1
        end
        before_update do
          @render_count = @render_count + 1
        end
        render(DIV) do
          # render the all scope first, which will create a unloaded/dummy collection for all
          UL { TestModel.send(TestComponent2.scope || :all).each { |test_model| LI { test_model.test_attribute }}}
          # now get the count of the quicker (client side) scope, which must NOT use the dummy all collection as a base
          DIV { "quicker.count = #{TestModel.quicker.count}" }
        end
      end
    end
    page.should have_content('.count = 2')
    evaluate_ruby do
      TestComponent2.scope = :all
    end
    page.should have_content('model-2')
    page.should have_content('model-4')
    page.should_not have_content('model-1', wait: 0)
    page.should_not have_content('model-3', wait: 0)
  end

@catmando
Copy link
Contributor Author

catmando commented Dec 7, 2018

note order is important if you move DIV { "quicker.count ... } first it works.

@catmando
Copy link
Contributor Author

catmando commented Dec 7, 2018

it appears that adding the following guard to the beginning of sync_collection_with_parent does the trick:

return if @parent.dummy_collection

@catmando
Copy link
Contributor Author

closed. for example see batch4/scope_spec.rb it 'the client filter will not be applied to unloaded scope but will be applied once the scope is loaded'

@catmando catmando added the ready-to-release Internal Use Only: Has been fixed, specs passing and pushed to edge branch label Jan 15, 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