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

double request when call eager loaded associations (owners, tasks, comments) #94

Open
drselump14 opened this issue Feb 13, 2017 · 1 comment
Assignees

Comments

@drselump14
Copy link
Contributor

In story resource, when we eager load association (owners, tasks, comments) and the association has empty value, then it'll try to fetch the information again. Is it an expected behavior?

def owners(params = {})
     if params.blank? && @owners.present?
        @owners
     else
        @owners = Endpoints::StoryOwners.new(client).get(project_id, id, params)
     end
 end

@forest
Copy link
Contributor

forest commented Feb 15, 2017

I can see how in your case that is not the desired behavior. I don't think it is right to build in knowledge of the eager loading in the client. I think a good solution is to keep the knowledge with the user and add a param that allows the user to force a load of data or not depending on their situation.

I think it should return the @owners if it is not undefined or nil. When you eager load and there are no owners then @owners should be an empty array. This is valid.

If params are passed then the load should be forced as well. Same behavior as today.

If @owners is an empty array and you want to force a load then maybe there should be a reload param that allows the user to reload the cache from the server.

The problem with the current code is that Virtus is nullifying blank (strings, arrays, etc) during coercion. This behavior is needed for the dirty checking feature. Not a simple fix.

I'm looking into replacing Virtus with some of the http://dry-rb.org/ libraries. I wrote a test for your case that is failing now and will make sure it passes in the rewrite.

@forest forest self-assigned this Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants