Prototype: process an additional param that is sent to associations #123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Method here is a simple extra param passed in; real effort should focus around allowing include to specify perPage for associations
urnf
commented
Jun 6, 2019
| if options[:lookup] | ||
| run_on_with_lookup(model, context, helper_instance) | ||
| elsif options[:dynamic] | ||
| proc = options[:dynamic] | ||
| if proc.arity == 1 | ||
| if proc.arity == -2 |
There was a problem hiding this comment.
Since the expected association proc format takes in model and options = {}, we start with arity 2, and with one of the params not required, makes it -2.
I'm not sure about the case at the bottom with no arity - as far as I know, our documentation requires that at least a model is passed in, resulting in arity 1?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE. MEANT FOR DISCUSSION.
Proof of concept meant to explore passing additional options to
dynamicassociations. While this is tailored towards the placeholder "firstFiftyAssociations ", meant to address a current problem with pagination, something like this would lay the groundwork for additional options.In the presenter, association definition goes from
to
The new param is not required to maintain backwards compatibility and leaves the responsibility of handling that option up to the association proc itself. The procs themselves would probably end up needing to be refactored wholesale to support any new features that would be added to them, such as pagination.