Heshamaly/add column selector extension #87
Conversation
Signed-off-by: Hesham Mohamed <[email protected]>
Signed-off-by: Hesham Mohamed <[email protected]>
HeshamaMohamed
force-pushed
the
heshamaly/add_column_selector_extension
branch
2 times, most recently
from
5ebff42 to
dd2b459
Compare
Signed-off-by: Hesham Mohamed <[email protected]>
HeshamaMohamed
force-pushed
the
heshamaly/add_column_selector_extension
branch
from
6b97423 to
7da75b1
Compare
|
This PR is stale because it has been open for 30 days with no activity and will be closed in 14 days unless you add a comment. |
|
Planning to review thoroughly soon |
| @values[:preload_blueprint_method] | ||
| end | ||
|
|
||
| def select_blueprint_columns_method |
There was a problem hiding this comment.
The _method suffix doesn't make sense IMO, since we're just storing a boolean. Maybe something like select_blueprint_columns?.
| @values[:before_preload_blueprint] = val | ||
| end | ||
|
|
||
| # Get the selects present before the ColumnSelector extension ran (internal, for logging) |
There was a problem hiding this comment.
I think we can get rid of these two methods, unless you're planning to add an optional, no-op logging extension like the Preloader has.
| # @param model [Class|:polymorphic] The ActiveRecord model class that blueprint represents | ||
| # @return [Array<String>] Array of column names to select | ||
| # | ||
| def self.columns(blueprint, view_name, model:) |
There was a problem hiding this comment.
Would be nice to break this method into some smaller ones.
| elsif ref.through_reflection | ||
| # For has_many/one through belongs_to associations, we need the foreign key of the intermediate association | ||
| # Example: has_many :customer_projects, through: :customer | ||
| # We need the customer_id foreign key |
There was a problem hiding this comment.
Will this handle multiple levels of "through" associations?
| end | ||
| end | ||
| end | ||
| # Databases often cache query plans. Consistent column order means: |
|
This PR is stale because it has been open for 30 days with no activity and will be closed in 14 days unless you add a comment. |
|
This PR was closed because it has been stalled for 14 days with no activity. |
|
This PR is stale because it has been open for 30 days with no activity and will be closed in 14 days unless you add a comment. |
2 participants
Checklist:
TODO:
Next iteration