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

fix: standardize behavior of make:cell and Cells #7481

Merged
merged 3 commits into from
May 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/Config/Generators.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class Generators extends BaseConfig
*/
public array $views = [
'make:cell' => 'CodeIgniter\Commands\Generators\Views\cell.tpl.php',
'make:cell_view' => 'CodeIgniter\Commands\Generators\Views\cell_view.tpl.php',
'make:command' => 'CodeIgniter\Commands\Generators\Views\command.tpl.php',
'make:config' => 'CodeIgniter\Commands\Generators\Views\config.tpl.php',
'make:controller' => 'CodeIgniter\Commands\Generators\Views\controller.tpl.php',
Expand Down
6 changes: 3 additions & 3 deletions system/CLI/GeneratorTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,15 @@ protected function qualifyClassName(): string
$component = singular($this->component);

/**
* @see https://regex101.com/r/a5KNCR/1
* @see https://regex101.com/r/a5KNCR/2
*/
$pattern = sprintf('/([a-z][a-z0-9_\/\\\\]+)(%s)/i', $component);
$pattern = sprintf('/([a-z][a-z0-9_\/\\\\]+)(%s)$/i', $component);

if (preg_match($pattern, $class, $matches) === 1) {
$class = $matches[1] . ucfirst($matches[2]);
}

if ($this->enabledSuffixing && $this->getOption('suffix') && ! strripos($class, $component)) {
if ($this->enabledSuffixing && $this->getOption('suffix') && preg_match($pattern, $class) !== 1) {
$class .= ucfirst($component);
}

Expand Down
28 changes: 13 additions & 15 deletions system/Commands/Generators/CellGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class CellGenerator extends BaseCommand
*/
protected $options = [
'--namespace' => 'Set root namespace. Default: "APP_NAMESPACE".',
'--suffix' => 'Append the component title to the class name (e.g. User => UserCell).',
'--force' => 'Force overwrite existing file.',
];

Expand All @@ -74,27 +73,26 @@ class CellGenerator extends BaseCommand
*/
public function run(array $params)
{
// Generate the Class first
$this->component = 'Cell';
$this->directory = 'Cells';
$this->component = 'Cell';
$this->directory = 'Cells';

$params = array_merge($params, ['suffix' => null]);

$this->template = 'cell.tpl.php';
$this->classNameLang = 'CLI.generator.className.cell';

$this->generateClass($params);

// Generate the View
$this->name = 'make:cell_view';
$this->template = 'cell_view.tpl.php';
$this->classNameLang = 'CLI.generator.viewName.cell';

// Form the view name
$segments = explode('\\', $this->qualifyClassName());

$view = array_pop($segments);
$view = decamelize($view);
$segments[] = $view;
$view = implode('\\', $segments);
$className = $this->qualifyClassName();
$viewName = decamelize(class_basename($className));
$viewName = preg_replace('/([a-z][a-z0-9_\/\\\\]+)(_cell)$/i', '$1', $viewName) ?? $viewName;
$namespace = substr($className, 0, strrpos($className, '\\') + 1);

$this->template = 'cell_view.tpl.php';
$this->generateView($namespace . $viewName, $params);

$this->generateView($view, $params);
return 0;
}
}
49 changes: 32 additions & 17 deletions system/View/Cells/Cell.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\View\Cells;

use CodeIgniter\Traits\PropertiesTrait;
use LogicException;
use ReflectionClass;

/**
Expand Down Expand Up @@ -64,37 +65,51 @@ public function setView(string $view)
* from within the view, this method extracts $data into the
* current scope and captures the output buffer instead of
* relying on the view service.
*
* @throws LogicException
*/
final protected function view(?string $view, array $data = []): string
{
$properties = $this->getPublicProperties();
$properties = $this->includeComputedProperties($properties);
$properties = array_merge($properties, $data);

// If no view is specified, we'll try to guess it based on the class name.
if (empty($view)) {
// According to the docs, the name of the view file should be the
// snake_cased version of the cell's class name, but for backward
// compatibility, the name also accepts '_cell' being omitted.
$ref = new ReflectionClass($this);
$view = decamelize($ref->getShortName());
$viewPath = dirname($ref->getFileName()) . DIRECTORY_SEPARATOR . $view . '.php';
$view = is_file($viewPath) ? $viewPath : str_replace('_cell', '', $view);
$view = (string) $view;

if ($view === '') {
$viewName = decamelize(class_basename(static::class));
$directory = dirname((new ReflectionClass($this))->getFileName()) . DIRECTORY_SEPARATOR;

$possibleView1 = $directory . substr($viewName, 0, strrpos($viewName, '_cell')) . '.php';
$possibleView2 = $directory . $viewName . '.php';
}

// Locate our view, preferring the directory of the class.
if (! is_file($view)) {
// Get the local pathname of the Cell
$ref = new ReflectionClass($this);
$view = dirname($ref->getFileName()) . DIRECTORY_SEPARATOR . $view . '.php';
if ($view !== '' && ! is_file($view)) {
$directory = dirname((new ReflectionClass($this))->getFileName()) . DIRECTORY_SEPARATOR;

$view = $directory . $view . '.php';
}

return (function () use ($properties, $view): string {
$candidateViews = array_filter(
[$view, $possibleView1 ?? '', $possibleView2 ?? ''],
static fn (string $path): bool => $path !== '' && is_file($path)
);

if ($candidateViews === []) {
throw new LogicException(sprintf(
'Cannot locate the view file for the "%s" cell.',
static::class
));
}

$foundView = current($candidateViews);

return (function () use ($properties, $foundView): string {
extract($properties);
ob_start();
include $view;
include $foundView;

return ob_get_clean() ?: '';
return ob_get_clean();
})();
}

Expand Down
18 changes: 18 additions & 0 deletions tests/_support/View/Cells/BadCell.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace Tests\Support\View\Cells;

use CodeIgniter\View\Cells\Cell;

final class BadCell extends Cell
{
}
39 changes: 29 additions & 10 deletions tests/system/Commands/CellGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,35 +46,54 @@ protected function getFileContents(string $filepath): string
return file_get_contents($filepath) ?: '';
}

public function testGenerateCell()
public function testGenerateCell(): void
{
command('make:cell RecentCell');

// Check the class was generated
$file = APPPATH . 'Cells/RecentCell.php';
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$contents = $this->getFileContents($file);
$this->assertStringContainsString('class RecentCell extends Cell', $contents);
$this->assertStringContainsString('class RecentCell extends Cell', $this->getFileContents($file));

// Check the view was generated
$file = APPPATH . 'Cells/recent_cell.php';
$this->assertStringContainsString('File created: ', $this->getStreamFilterBuffer());
$file = APPPATH . 'Cells/recent.php';
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$this->assertSame("<div>\n <!-- Your HTML here -->\n</div>\n", $this->getFileContents($file));
}

public function testGenerateCellSimpleName()
public function testGenerateCellSimpleName(): void
{
command('make:cell Another');

// Check the class was generated
$file = APPPATH . 'Cells/Another.php';
$file = APPPATH . 'Cells/AnotherCell.php';
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$contents = $this->getFileContents($file);
$this->assertStringContainsString('class Another extends Cell', $contents);
$this->assertStringContainsString('class AnotherCell extends Cell', $this->getFileContents($file));

// Check the view was generated
$file = APPPATH . 'Cells/another.php';
$this->assertStringContainsString('File created: ', $this->getStreamFilterBuffer());
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$this->assertSame("<div>\n <!-- Your HTML here -->\n</div>\n", $this->getFileContents($file));
}

public function testGenerateCellWithCellInBetween(): void
{
command('make:cell PippoCellular');

// Check the class was generated
$file = APPPATH . 'Cells/PippoCellularCell.php';
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$this->assertStringContainsString('class PippoCellularCell extends Cell', $this->getFileContents($file));

// Check the view was generated
$file = APPPATH . 'Cells/pippo_cellular.php';
$this->assertStringContainsString('File created: ' . clean_path($file), $this->getStreamFilterBuffer());
$this->assertFileExists($file);
$this->assertSame("<div>\n <!-- Your HTML here -->\n</div>\n", $this->getFileContents($file));
}
}
10 changes: 10 additions & 0 deletions tests/system/View/ControlledCellTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@

use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\View\Exceptions\ViewException;
use LogicException;
use Tests\Support\View\Cells\AdditionCell;
use Tests\Support\View\Cells\AwesomeCell;
use Tests\Support\View\Cells\BadCell;
use Tests\Support\View\Cells\ColorsCell;
use Tests\Support\View\Cells\GreetingCell;
use Tests\Support\View\Cells\ListerCell;
Expand Down Expand Up @@ -65,6 +67,14 @@ public function testCellThroughRenderMethodWithExtraData()
$this->assertStringContainsString('42, 23, 16, 15, 8, 4', $result);
}

public function testCellThrowsExceptionWhenCannotFindTheViewFile()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot locate the view file for the "Tests\\Support\\View\\Cells\\BadCell" cell.');

view_cell(BadCell::class);
}

public function testCellWithParameters()
{
$result = view_cell(GreetingCell::class, 'greeting=Hi, name=CodeIgniter');
Expand Down
8 changes: 8 additions & 0 deletions user_guide_src/source/changelogs/v4.3.5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@ Message Changes
Changes
*******

- **make:cell** When creating a new cell, the controller would always have the ``Cell`` suffixed to the class name.
For the view file, the final ``_cell`` is always removed.
- **Cells** For compatibility with previous versions, view filenames ending with ``_cell`` can still be
located by the ``Cell`` as long as auto-detection of view file is enabled (via setting the ``$view`` property
to an empty string).

Deprecations
************

Bugs Fixed
**********

- **Validation:** Fixed a bug where a closure used in combination with ``permit_empty`` or ``if_exist`` rules was causing an error.
- **make:cell** Fixed generating view files as classes.
- **make:cell** Fixed treatment of single word class input for case-insensitive OS.

See the repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
Expand Down
3 changes: 1 addition & 2 deletions user_guide_src/source/cli/cli_generators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,11 @@ Usage:

Argument:
=========
* ``name``: The name of the cell class. It should be in PascalCase.
* ``name``: The name of the cell class. It should be in PascalCase. **[REQUIRED]**

Options:
========
* ``--namespace``: Set the root namespace. Defaults to value of ``APP_NAMESPACE``.
* ``--suffix``: Append the component suffix to the generated class name.
* ``--force``: Set this flag to overwrite existing files on destination.

make:command
Expand Down
15 changes: 11 additions & 4 deletions user_guide_src/source/outgoing/view_cells.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,14 @@ Controlled Cells

.. versionadded:: 4.3.0

Controlled Cells have two primary goals: to make it as fast as possible to build the cell, and provide additional logic and flexibility to your views, if they need it. The class must extend ``CodeIgniter\View\Cells\Cell``. They should have a view file in the same folder. By convention the class name should be PascalCase and the view should be the snake_cased version of the class name. So, for example, if you have a ``MyCell`` class, the view file should be ``my_cell.php``.
Controlled cells have two primary goals: to make it as fast as possible to build the cell, and provide additional logic and
flexibility to your views, if they need it. The class must extend ``CodeIgniter\View\Cells\Cell``. They should have a view file
in the same folder. By convention, the class name should be in PascalCase suffixed with ``Cell`` and the view should be
the snake_cased version of the class name, without the suffix. For example, if you have a ``MyCell`` class, the view file
should be ``my.php``.

.. note:: Prior to v4.3.5, the generated view file ends with ``_cell.php``. Though v4.3.5 and newer will generate view files
without the ``_cell`` suffix, existing view files will still be located and loaded.

Creating a Controlled Cell
==========================
Expand All @@ -99,7 +106,7 @@ At the most basic level, all you need to implement within the class are public p
public $message;
}

// app/Cells/alert_message_cell.php
// app/Cells/alert_message.php
<div class="alert alert-<?= esc($type, 'attr') ?>">
<?= esc($message) ?>
</div>
Expand Down Expand Up @@ -199,7 +206,7 @@ If you need to perform additional logic for one or more properties you can use c
}
}

// app/Cells/alert_message_cell.php
// app/Cells/alert_message.php
<div>
<p>type - <?= esc($type) ?></p>
<p>message - <?= esc($message) ?></p>
Expand Down Expand Up @@ -230,7 +237,7 @@ Sometimes you need to perform additional logic for the view, but you don't want
}
}

// app/Cells/recent_posts_cell.php
// app/Cells/recent_posts.php
<ul>
<?php foreach ($posts as $post): ?>
<li><?= $this->linkPost($post) ?></li>
Expand Down