Skip to content

[5.4] Better code for save2copy Smart Search Filter #46449

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.

Sign up for GitHub

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

Closed
joomdonation wants to merge 1 commit into from
Closed

[5.4] Better code for save2copy Smart Search Filter #46449

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 21 additions & 99 deletions administrator/components/com_finder/src/Model/FilterModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
namespace Joomla\Component\Finder\Administrator\Model;

use Joomla\CMS\Factory;
use Joomla\CMS\Filter\OutputFilter;
use Joomla\CMS\Form\Form;
use Joomla\CMS\MVC\Model\AdminModel;
use Joomla\Component\Finder\Administrator\Table\FilterTable;
Expand Down Expand Up @@ -167,123 +166,46 @@ public function getTotal()
*/
public function save($data)
{
$app = Factory::getApplication();
$task = $app->getInput()->get('task', '', 'cmd');
if ($this->getState('task') === 'save2copy') {
$table = $this->getTable();
$table->load(Factory::getApplication()->getInput()->getInt('filter_id', 0));
Copy link
Member

@laoneo laoneo Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filter_id id should be passed from the controller's save function to here as there is the input object available.

Copy link
Contributor Author

@joomdonation joomdonation Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laoneo For that, we should find a way to handle it in save method of FormController https://github.com/joomla/joomla-cms/blob/5.4-dev/libraries/src/MVC/Controller/FormController.php#L575 instead of having to do that in every controller. As this is a bug fix release, I don't want to touch the our MVC code.

Copy link
Member

@laoneo laoneo Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this pr, you can do it here https://github.com/joomla/joomla-cms/blob/5.4-dev/administrator/components/com_finder/src/Controller/FilterController.php#L152

Copy link
Contributor Author

@joomdonation joomdonation Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I can do that. But it is just a workaround. Later (in 6.1 for example), we might decide to pass ID of the source the record uses a state variable instead of via $data array, and in that case, we will need to update the code here again.

Copy link
Member

@laoneo laoneo Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a workaround, the controller is where you should deal with user input.

Copy link
Contributor Author

@joomdonation joomdonation Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. But as I said, we should find a way to handle it in FormController so that we have to pass that data in each child controller.

Copy link
Contributor Author

@joomdonation joomdonation Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, I will stop doing change until we have a final decision. Look like maintainers want to revert the original PR and fix/handle it properly in 6.1


if ($task === 'save2copy') {
$data['filter_id'] = 0;
if ($table->title === $data['title']) {
[$title, $alias] = $this->generateNewFilterTitleAndAlias($data['alias'], $data['title']);

$title = trim((string) ($data['title'] ?? ''));
$alias = trim((string) ($data['alias'] ?? ''));

if ($alias === '') {
$alias = OutputFilter::stringURLSafe($title);
$data['title'] = $title;
$data['alias'] = $alias;
} elseif ($table->alias === $data['alias']) {
$data['alias'] = '';
}

// Generate unique title + alias
list($title, $alias) = $this->generateNewTitleAndAlias($title, $alias);

$data['title'] = $title;
$data['alias'] = $alias;
}

return parent::save($data);
}

/**
* Generate a new unique title and alias for a copied filter.
* Follows the same logic as Joomla's core content models.
* Generate new title and alias to make sure alias is unique when filter is created using save2copy
*
* @param string $title The original title.
* @param string $alias The original alias.
* @param string $alias The alias.
* @param string $title The title.
*
* @return array Array with [newTitle, newAlias].
* @return array Contains the new title and alias.
*
* @since 5.4.1
* @since __DEPLOY_VERSION__
*/
protected function generateNewTitleAndAlias(string $title, string $alias): array
protected function generateNewFilterTitleAndAlias(string $alias, string $title): array
{
$db = $this->getDatabase();

if (preg_match('/^(.*?)(?:\s\((\d+)\))?$/', $title, $matches)) {
$baseTitle = trim($matches[1]);
} else {
$baseTitle = trim($title);
}

$baseAlias = trim($alias ?: OutputFilter::stringURLSafe($title));

$likeTitle = $db->quote($db->escape($baseTitle, true) . '%', false);

$query = $db->getQuery(true)
->select($db->quoteName('title'))
->from($db->quoteName('#__finder_filters'))
->where($db->quoteName('title') . ' LIKE ' . $likeTitle);
// Alter the title & alias
$table = $this->getTable();

$existingTitles = $db->setQuery($query)->loadColumn();

$maxNum = 0;
foreach ($existingTitles as $existing) {
if (preg_match('/^\Q' . $baseTitle . '\E(?:\s\((\d+)\))?$/', $existing, $matches)) {
$num = isset($matches[1]) ? (int) $matches[1] : 1;
if ($num > $maxNum) {
$maxNum = $num;
}
while ($table->load(['alias' => $alias])) {
if ($title === $table->title) {
$title = StringHelper::increment($title);
}
}

$nextNum = $maxNum + 1;

$newTitle = $baseTitle;
if ($nextNum > 1) {
$newTitle .= ' (' . $nextNum . ')';
}

// Build a unique alias
$newAlias = $this->getUniqueAlias($baseAlias);

return [$newTitle, $newAlias];
}

/**
* Ensure a unique alias in the table by incrementing with dash style.
*
*
* @param string $base The starting alias (already URL-safe).
*
* @return string A unique alias.
*
* @since 5.4.1
*/
protected function getUniqueAlias(string $base): string
{
$alias = $base !== '' ? $base : OutputFilter::stringURLSafe(uniqid('filter-', true));

while ($this->aliasExists($alias)) {
$alias = StringHelper::increment($alias, 'dash');
}

return $alias;
}

/**
* Check whether an alias exists in the table.
*
* @param string $alias The alias to test.
*
* @return boolean True if it exists, false otherwise.
*
* @since 5.4.1
*/
protected function aliasExists(string $alias): bool
{
$db = $this->getDatabase();
$query = $db->getQuery(true)
->select('COUNT(*)')
->from($db->quoteName('#__finder_filters'))
->where($db->quoteName('alias') . ' = :alias')
->bind(':alias', $alias);

return (int) $db->setQuery($query)->loadResult() > 0;
return [$title, $alias];
}
}