From 0a196848c14178dc5f480d96992420f3d8dbc8f6 Mon Sep 17 00:00:00 2001 From: Bryan Nielsen Date: Mon, 9 Oct 2023 14:53:43 -0400 Subject: [PATCH 1/3] Add 'with' parameter to channel entries tag --- CHANGELOG.md | 1 + src/Support/Arguments/FilterArgument.php | 6 +++--- src/View/ModelTag.php | 16 ++++++++++++++++ src/View/Tags/Channel/Entries.php | 11 +++++++++++ 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f61d315..ed1881f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - Artisan command to proxy ExpressionEngine's `eecli` commands in the context of Laravel and Coilpack after they have fully booted. This command is available through `php artisan eecli`. +- Parameter to Channel Entries Tag for eager loading relationships. This allows for easier inclusion of custom field data with `exp.channel.entries({with: 'data'})` ### Fixed - Behavior of File Modifier to return original data when manipulation fails silently diff --git a/src/Support/Arguments/FilterArgument.php b/src/Support/Arguments/FilterArgument.php index 3247c32..abcd79a 100644 --- a/src/Support/Arguments/FilterArgument.php +++ b/src/Support/Arguments/FilterArgument.php @@ -28,13 +28,13 @@ public function __construct($value) protected function parse($value) { // Check for a negated set - if (strncasecmp($value, 'not ', 4) == 0) { + if (strncasecmp($value ?? '', 'not ', 4) == 0) { $this->not = true; $value = substr($value, 4); } // Check for set joined by logical and - if (strpos($value, '&&')) { + if (strpos($value ?? '', '&&')) { $this->separator = '&&'; $this->boolean = 'and'; } @@ -44,7 +44,7 @@ protected function parse($value) $this->boolean = ($this->boolean === 'or') ? 'and' : 'or'; } - $terms = array_filter(explode($this->separator, $value), function ($term) { + $terms = array_filter(explode($this->separator, $value ?? ''), function ($term) { return ! is_null($term) && trim($term) !== ''; }); diff --git a/src/View/ModelTag.php b/src/View/ModelTag.php index 460d41f..7e30db2 100644 --- a/src/View/ModelTag.php +++ b/src/View/ModelTag.php @@ -3,6 +3,7 @@ namespace Expressionengine\Coilpack\View; use Expressionengine\Coilpack\Support\Parameter; +use Expressionengine\Coilpack\Support\Arguments\ListArgument; abstract class ModelTag extends IterableTag { @@ -35,9 +36,20 @@ public function defineParameters(): array 'description' => 'How many results to show on each page', 'defaultValue' => 10, ]), + new Parameter([ + 'name' => 'with', + 'type' => 'string', + 'description' => 'A pipe separated list of relationships to eager load', + 'defaultValue' => null, + ]), ]; } + public function getWithArgument($value) + { + return new ListArgument($value); + } + public function run() { if ($this->hasArgument('page') || $this->hasArgument('per_page')) { @@ -57,6 +69,10 @@ public function run() $this->query->take($this->getArgument('limit')->value); } + if ($this->hasArgument('with')) { + $this->query->with($this->getArgument('with')->terms->map->value->toArray()); + } + return $this->query->get(); } diff --git a/src/View/Tags/Channel/Entries.php b/src/View/Tags/Channel/Entries.php index ffe3764..c1477b5 100644 --- a/src/View/Tags/Channel/Entries.php +++ b/src/View/Tags/Channel/Entries.php @@ -190,6 +190,17 @@ public function getSearchArgument($search) return $search; } + public function getWithArgument($value) + { + // If the 'data' relation is requested we will append 'channel' and 'hiddenFields' + // because these are necessary for properly displaying custom field data + if(in_array('data', explode('|', $value))) { + $value .= '|channel|hiddenFields'; + } + + return parent::getWithArgument($value); + } + public function run() { // Author From b8c44170e704f4e453317952c93fd136f31a2f3e Mon Sep 17 00:00:00 2001 From: Bryan Nielsen Date: Tue, 17 Oct 2023 07:53:41 -0400 Subject: [PATCH 2/3] Formatting and code cleanup --- src/Api/Graph/Support/FieldtypeRegistrar.php | 6 ++--- src/Bootstrap/LoadExpressionEngine.php | 2 +- src/Commands/EECliCommand.php | 24 +++++++++----------- src/Core.php | 1 - src/Fieldtypes/Generic.php | 2 +- src/Fieldtypes/Modifiers/File.php | 4 ++-- src/Traits/CanAccessRestrictedClass.php | 7 +++--- src/View/ModelTag.php | 2 +- src/View/Tags/Channel/Entries.php | 2 +- 9 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/Api/Graph/Support/FieldtypeRegistrar.php b/src/Api/Graph/Support/FieldtypeRegistrar.php index 238df48..16fe3aa 100644 --- a/src/Api/Graph/Support/FieldtypeRegistrar.php +++ b/src/Api/Graph/Support/FieldtypeRegistrar.php @@ -107,15 +107,15 @@ public function registerField(ChannelField $field) return $this->types[$field->field_name]; } - if (!$fieldtype instanceof GeneratesGraphType) { + if (! $fieldtype instanceof GeneratesGraphType) { return null; } $name = "Field__{$field->field_name}"; $typeDefinition = $fieldtype->generateGraphType($field); - if(!$typeDefinition instanceof GraphQLType) { - throw new \Exception("Generated GraphQL Type for field {$field->field_name} must extend ".GraphQLType::class."."); + if (! $typeDefinition instanceof GraphQLType) { + throw new \Exception("Generated GraphQL Type for field {$field->field_name} must extend ".GraphQLType::class.'.'); } $typeDefinition->name = $name; diff --git a/src/Bootstrap/LoadExpressionEngine.php b/src/Bootstrap/LoadExpressionEngine.php index d5173d5..a5da2c9 100644 --- a/src/Bootstrap/LoadExpressionEngine.php +++ b/src/Bootstrap/LoadExpressionEngine.php @@ -67,7 +67,7 @@ public function cli() // fake SERVER vars for CLI context $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; $this->constants['REQ'] = 'CLI'; - $this->constants['EE_INSTALLED'] = file_exists($this->constants['SYSPATH'] . 'user/config/config.php'); + $this->constants['EE_INSTALLED'] = file_exists($this->constants['SYSPATH'].'user/config/config.php'); return $this; } diff --git a/src/Commands/EECliCommand.php b/src/Commands/EECliCommand.php index 74a7f47..19b7509 100644 --- a/src/Commands/EECliCommand.php +++ b/src/Commands/EECliCommand.php @@ -7,12 +7,11 @@ class EECliCommand extends Command { - use CanAccessRestrictedClass; public $signature = 'eecli {args?*}'; - public $description = "Run ExpressionEngine commands through Coilpack"; + public $description = 'Run ExpressionEngine commands through Coilpack'; private $cli; @@ -20,7 +19,7 @@ public function __construct() { parent::__construct(); - if(($_SERVER['argv'][1] ?? '') != 'eecli') { + if (($_SERVER['argv'][1] ?? '') != 'eecli') { return; } @@ -29,10 +28,10 @@ public function __construct() $this->setupCommand($_SERVER['argv'][2] ?? 'list'); - foreach($_SERVER['argv'] as $arg) { + foreach ($_SERVER['argv'] as $arg) { // If we have a help flag we need to get out early so that it can be // handled by the ExpressionEngine Command and not by Symfony/Laravel - if($arg == '--help') { + if ($arg == '--help') { return $this->handle(); } } @@ -42,7 +41,7 @@ protected function setupCommand($command) { $availableCommands = $this->callRestrictedMethod($this->cli, 'availableCommands'); - if (!array_key_exists($command, $availableCommands)) { + if (! array_key_exists($command, $availableCommands)) { exit("Command '$command' not found."); } @@ -61,23 +60,22 @@ protected function setupCommand($command) $this->callRestrictedMethod($commandClass, 'loadOptions'); // Convert ExpressionEngine Command's option syntax into Laravel's and update the signature $options = $this->getRestrictedProperty($commandClass, 'commandOptions'); - $this->signature = $this->signature . ' ' . implode(' ', array_map(function($option) { - if(strpos($option, 'verbose') !== false) { + $this->signature = $this->signature.' '.implode(' ', array_map(function ($option) { + if (strpos($option, 'verbose') !== false) { return ''; } $pieces = explode(',', $option); - if(count($pieces) == 2) { - $option = substr($pieces[1], 0, 1) . '|' . $pieces[0]; + if (count($pieces) == 2) { + $option = substr($pieces[1], 0, 1).'|'.$pieces[0]; } - return "{--".$option."=}"; + + return '{--'.$option.'=}'; }, array_keys($options))); } - public function handle(): int { return $this->cli->process(); } - } diff --git a/src/Core.php b/src/Core.php index c87b615..11dbe54 100644 --- a/src/Core.php +++ b/src/Core.php @@ -4,7 +4,6 @@ class Core { - use Traits\CanAccessRestrictedClass; protected $core; diff --git a/src/Fieldtypes/Generic.php b/src/Fieldtypes/Generic.php index 63523df..feea8eb 100644 --- a/src/Fieldtypes/Generic.php +++ b/src/Fieldtypes/Generic.php @@ -61,7 +61,7 @@ public function apply(FieldContent $content, array $parameters = []) $data = $content->getAttribute('data'); // Run pre_process if it exists - if(method_exists($handler, 'pre_process')) { + if (method_exists($handler, 'pre_process')) { $data = $handler->pre_process($data); } diff --git a/src/Fieldtypes/Modifiers/File.php b/src/Fieldtypes/Modifiers/File.php index c81d213..2de5f8e 100644 --- a/src/Fieldtypes/Modifiers/File.php +++ b/src/Fieldtypes/Modifiers/File.php @@ -22,9 +22,9 @@ public function handle(FieldtypeOutput $content, $parameters = []) $modified = (is_array($modified) && count($modified) === 1 && is_array(current($modified))) ? $modified[0] : $modified; // If a manipulation fails and returns a boolean we fallback to the original url - if(is_bool($modified)) { + if (is_bool($modified)) { $modified = [ - 'url' => $data['url'] ?? '' + 'url' => $data['url'] ?? '', ]; } diff --git a/src/Traits/CanAccessRestrictedClass.php b/src/Traits/CanAccessRestrictedClass.php index 26502de..3b10ba2 100644 --- a/src/Traits/CanAccessRestrictedClass.php +++ b/src/Traits/CanAccessRestrictedClass.php @@ -2,8 +2,8 @@ namespace Expressionengine\Coilpack\Traits; -trait CanAccessRestrictedClass { - +trait CanAccessRestrictedClass +{ protected function getRestrictedProperty($object, $property) { $reflection = new \ReflectionClass($object); @@ -28,5 +28,4 @@ public function callRestrictedMethod($object, $method, $parameters = []) return $method->invoke($object, ...$parameters); } - -} \ No newline at end of file +} diff --git a/src/View/ModelTag.php b/src/View/ModelTag.php index 7e30db2..c7e03a1 100644 --- a/src/View/ModelTag.php +++ b/src/View/ModelTag.php @@ -2,8 +2,8 @@ namespace Expressionengine\Coilpack\View; -use Expressionengine\Coilpack\Support\Parameter; use Expressionengine\Coilpack\Support\Arguments\ListArgument; +use Expressionengine\Coilpack\Support\Parameter; abstract class ModelTag extends IterableTag { diff --git a/src/View/Tags/Channel/Entries.php b/src/View/Tags/Channel/Entries.php index c1477b5..78def7c 100644 --- a/src/View/Tags/Channel/Entries.php +++ b/src/View/Tags/Channel/Entries.php @@ -194,7 +194,7 @@ public function getWithArgument($value) { // If the 'data' relation is requested we will append 'channel' and 'hiddenFields' // because these are necessary for properly displaying custom field data - if(in_array('data', explode('|', $value))) { + if (in_array('data', explode('|', $value))) { $value .= '|channel|hiddenFields'; } From 4f84b71ae01c5f483744202135d242205b0a0487 Mon Sep 17 00:00:00 2001 From: Bryan Nielsen Date: Wed, 18 Oct 2023 11:23:52 -0400 Subject: [PATCH 3/3] Fix custom field orderby with legacy field data --- CHANGELOG.md | 1 + src/Models/Addon/Fluid/Data.php | 2 +- src/Models/Category/Category.php | 25 ++++++++++++++----------- src/Models/Category/CategoryData.php | 2 +- src/Models/Channel/ChannelData.php | 2 +- src/Models/Channel/ChannelEntry.php | 24 +++++++++++++----------- src/Models/Channel/ChannelField.php | 5 +++++ src/Models/Member/MemberData.php | 2 +- src/Models/Member/MemberField.php | 5 +++++ src/Traits/DataAcrossTables.php | 2 +- 10 files changed, 43 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed1881f..c7f0fc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Generic fieldtype passes `content_id` along to core fieldtype handler and triggers `pre_process` hook. - Error checking and handling for GraphQL compatible fieldtypes - Trigger `core_boot` hook during GraphQL requests +- Custom field orderby clauses when it uses legacy field data storage ## [1.1.2] - 2023-07-27 diff --git a/src/Models/Addon/Fluid/Data.php b/src/Models/Addon/Fluid/Data.php index c36f421..b535d4d 100644 --- a/src/Models/Addon/Fluid/Data.php +++ b/src/Models/Addon/Fluid/Data.php @@ -42,7 +42,7 @@ public function scopeCustomFields($query, $fields = null) // Get a set of table names for fields that do not store data on the legacy table $fields = $fields->filter(function ($field) { - return $field->legacy_field_data === 'n' || $field->legacy_field_data === false || $field->hasDataTable(); + return ! $field->hasLegacyFieldData() || $field->hasDataTable(); }); // Join these extra field data tables diff --git a/src/Models/Category/Category.php b/src/Models/Category/Category.php index fa0f9fb..823386c 100644 --- a/src/Models/Category/Category.php +++ b/src/Models/Category/Category.php @@ -4,6 +4,7 @@ use Expressionengine\Coilpack\FieldtypeManager; use Expressionengine\Coilpack\Model; +use Expressionengine\Coilpack\Models\Channel\ChannelData; use Expressionengine\Coilpack\Models\Channel\ChannelEntry; use Expressionengine\Coilpack\Models\FieldContent; @@ -46,28 +47,30 @@ public function scopeOrderByCustomField($query, $field, $direction = 'asc') $field = $manager->getField($field, 'category'); $column = "field_id_{$field->field_id}"; - // If this field is not storing it's data on the category_field_data table we - // will join the separate data table with a unique orderby_field_name alias - if ($field->legacy_field_data === 'n' || $field->legacy_field_data === false) { - $alias = "orderby_{$field->field_name}"; - $column = "$alias.$column"; - $this->scopeJoinFieldDataTable($query, $field, $alias); - } + // Setup our join for the appropriate channel data table and alias + $alias = $field->hasLegacyFieldData() ? 'orderby_legacy_data' : "orderby_{$field->field_name}"; + $this->scopeJoinFieldDataTable($query, $field, $alias); + $query->select($this->qualifyColumn('*')); - return $query->orderBy($column, $direction); + return $query->orderBy("$alias.$column", $direction); } public function scopeJoinFieldDataTable($query, $field, $alias = null) { - if ($field->legacy_field_data == 'y' || $field->legacy_field_data === true) { + // If we have already joined this table alias just do nothing + $alreadyJoined = collect($query->getQuery()->joins)->pluck('table')->contains(function ($joinTable) use ($alias) { + return strpos($joinTable, $alias) !== false; + }); + + if ($alreadyJoined) { return $query; } - $table = $field->data_table_name; + $table = $field->hasLegacyFieldData() ? (new ChannelData)->getTable() : $field->data_table_name; $joinTable = $alias ? "$table as $alias" : $table; $alias = $alias ?: $table; $query->leftJoin($joinTable, "$alias.cat_id", '=', $this->qualifyColumn('cat_id')); - $query->select('*', $this->qualifyColumn('cat_id')); + $query->select($this->qualifyColumn('*')); return $query; } diff --git a/src/Models/Category/CategoryData.php b/src/Models/Category/CategoryData.php index 5389189..3d384cf 100644 --- a/src/Models/Category/CategoryData.php +++ b/src/Models/Category/CategoryData.php @@ -32,7 +32,7 @@ public function scopeCustomFields($query, $fields = null) // Get a set of table names for fields that do not store data on the legacy table $fieldtypeTables = $fields->filter(function ($field) { - return $field->legacy_field_data === 'n' || $field->legacy_field_data === false; + return ! $field->hasLegacyFieldData(); })->map(function ($field) { return $field->data_table_name; }); diff --git a/src/Models/Channel/ChannelData.php b/src/Models/Channel/ChannelData.php index 97dfd16..af441ce 100644 --- a/src/Models/Channel/ChannelData.php +++ b/src/Models/Channel/ChannelData.php @@ -32,7 +32,7 @@ public function scopeCustomFields($query, $fields = null) // Get a set of table names for fields that do not store data on the legacy table $fieldtypeTables = $fields->filter(function ($field) { - return $field->legacy_field_data === 'n' || $field->legacy_field_data === false; + return ! $field->hasLegacyFieldData(); })->map(function ($field) { return $field->data_table_name; }); diff --git a/src/Models/Channel/ChannelEntry.php b/src/Models/Channel/ChannelEntry.php index 8715f51..621b62f 100644 --- a/src/Models/Channel/ChannelEntry.php +++ b/src/Models/Channel/ChannelEntry.php @@ -132,28 +132,30 @@ public function scopeOrderByCustomField($query, $field, $direction = 'asc') $field = $manager->getField($field); $column = "field_id_{$field->field_id}"; - // If this field is not storing it's data on the channel_data table we - // will join the separate data table with a unique orderby_field_name alias - if ($field->legacy_field_data === 'n' || $field->legacy_field_data === false) { - $alias = "orderby_{$field->field_name}"; - $column = "$alias.$column"; - $this->scopeJoinFieldDataTable($query, $field, $alias); - } + // Setup our join for the appropriate channel data table and alias + $alias = $field->hasLegacyFieldData() ? 'orderby_legacy_data' : "orderby_{$field->field_name}"; + $this->scopeJoinFieldDataTable($query, $field, $alias); + $query->select($this->qualifyColumn('*')); - return $query->orderBy($column, $direction); + return $query->orderBy("$alias.$column", $direction); } public function scopeJoinFieldDataTable($query, $field, $alias = null) { - if ($field->legacy_field_data == 'y' || $field->legacy_field_data === true) { + // If we have already joined this table alias just do nothing + $alreadyJoined = collect($query->getQuery()->joins)->pluck('table')->contains(function ($joinTable) use ($alias) { + return strpos($joinTable, $alias) !== false; + }); + + if ($alreadyJoined) { return $query; } - $table = $field->data_table_name; + $table = $field->hasLegacyFieldData() ? (new ChannelData)->getTable() : $field->data_table_name; $joinTable = $alias ? "$table as $alias" : $table; $alias = $alias ?: $table; $query->leftJoin($joinTable, "$alias.entry_id", '=', $this->qualifyColumn('entry_id')); - $query->select('*', $this->qualifyColumn('entry_id')); + $query->select($this->qualifyColumn('*')); return $query; } diff --git a/src/Models/Channel/ChannelField.php b/src/Models/Channel/ChannelField.php index ee3e98e..633d739 100644 --- a/src/Models/Channel/ChannelField.php +++ b/src/Models/Channel/ChannelField.php @@ -75,4 +75,9 @@ public function getGraphType() { return $this->getFieldType()->getGraphType($this); } + + public function hasLegacyFieldData() + { + return $this->legacy_field_data === 'y' || $this->legacy_field_data === true; + } } diff --git a/src/Models/Member/MemberData.php b/src/Models/Member/MemberData.php index edc6f99..eb801c7 100644 --- a/src/Models/Member/MemberData.php +++ b/src/Models/Member/MemberData.php @@ -33,7 +33,7 @@ public function scopeCustomFields($query, $fields = null) // Get a set of table names for fields that do not store data on the legacy table $fieldtypeTables = $fields->filter(function ($field) { - return $field->m_legacy_field_data === false || $field->m_legacy_field_data === 'n'; + return ! $field->hasLegacyFieldData(); })->map(function ($field) { return $field->data_table_name; }); diff --git a/src/Models/Member/MemberField.php b/src/Models/Member/MemberField.php index e679d83..1aa1a4a 100644 --- a/src/Models/Member/MemberField.php +++ b/src/Models/Member/MemberField.php @@ -36,4 +36,9 @@ public function getDataTableNameAttribute($value) { return "member_data_field_{$this->m_field_id}"; } + + public function hasLegacyFieldData() + { + return $this->m_legacy_field_data === 'y' || $this->m_legacy_field_data === true; + } } diff --git a/src/Traits/DataAcrossTables.php b/src/Traits/DataAcrossTables.php index b6a8e49..2c6648c 100644 --- a/src/Traits/DataAcrossTables.php +++ b/src/Traits/DataAcrossTables.php @@ -20,7 +20,7 @@ public function scopeCustomFields($query, $fields = null) // Get a set of table names for fields that do not store data on the legacy table $fieldtypeTables = $fields->filter(function ($field) { - return $field->legacy_field_data === 'n' || $field->legacy_field_data === false; + return ! $field->hasLegacyFieldData(); })->map(function ($field) { return $field->data_table_name; });