Skip to content

Refactor entities and operators #382

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

Merged
iMattPro merged 1 commit into from
Sep 24, 2026
Merged

Refactor entities and operators #382

Show file tree
Hide file tree
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
10 changes: 5 additions & 5 deletions config/services.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ services:
class: phpbb\boardrules\controller\admin_controller
arguments:
- '@config'
- '@service_container'
- '@controller.helper'
- '@language'
- '@language.loader'
Expand All @@ -44,20 +43,21 @@ services:
- '%core.root_path%'
- '%core.php_ext%'

phpbb.boardrules.entity:
class: phpbb\boardrules\entity\rule
shared: false # service MUST not be shared for this to work!
phpbb.boardrules.entity_factory:
class: phpbb\boardrules\entity\factory
arguments:
- '@dbal.conn'
- '%phpbb.boardrules.tables.boardrules%'

phpbb.boardrules.operator:
class: phpbb\boardrules\operators\rule
arguments:
- '@service_container'
- '@phpbb.boardrules.entity_factory'
- '@dbal.conn'
- '@phpbb.boardrules.nestedset_rules'
- '@phpbb.boardrules.ruleset_operator'
- '@phpbb.boardrules.table_lock'
- '%phpbb.boardrules.tables.boardrules%'

phpbb.boardrules.ruleset_operator:
class: phpbb\boardrules\operators\ruleset
Expand Down
137 changes: 84 additions & 53 deletions controller/admin_controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@

namespace phpbb\boardrules\controller;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* Admin controller
*/
Expand All @@ -20,9 +18,6 @@ class admin_controller implements admin_interface
/** @var \phpbb\config\config */
protected $config;

/** @var ContainerInterface */
protected $container;

/** @var \phpbb\controller\helper */
protected $controller_helper;

Expand Down Expand Up @@ -66,7 +61,6 @@ class admin_controller implements admin_interface
* Constructor
*
* @param \phpbb\config\config $config Config object
* @param ContainerInterface $container Service container interface
* @param \phpbb\controller\helper $controller_helper Controller helper object
* @param \phpbb\language\language $lang Language object
* @param \phpbb\language\language_file_loader $language_loader Language file loader
Expand All @@ -81,10 +75,9 @@ class admin_controller implements admin_interface
* @param string $php_ext phpEx
* @access public
*/
public function __construct(\phpbb\config\config $config, ContainerInterface $container, \phpbb\controller\helper $controller_helper, \phpbb\language\language $lang, \phpbb\language\language_file_loader $language_loader, \phpbb\log\log $log, \phpbb\notification\manager $notification_manager, \phpbb\request\request $request, \phpbb\boardrules\operators\rule $rule_operator, \phpbb\boardrules\operators\ruleset $ruleset_operator, \phpbb\template\template $template, \phpbb\user $user, $root_path, $php_ext)
public function __construct(\phpbb\config\config $config, \phpbb\controller\helper $controller_helper, \phpbb\language\language $lang, \phpbb\language\language_file_loader $language_loader, \phpbb\log\log $log, \phpbb\notification\manager $notification_manager, \phpbb\request\request $request, \phpbb\boardrules\operators\rule $rule_operator, \phpbb\boardrules\operators\ruleset $ruleset_operator, \phpbb\template\template $template, \phpbb\user $user, $root_path, $php_ext)
{
$this->config = $config;
$this->container = $container;
$this->controller_helper = $controller_helper;
$this->lang = $lang;
$this->language_loader = $language_loader;
Expand Down Expand Up @@ -234,7 +227,6 @@ public function display_language_dashboard()
* @param int $parent_id Category to display rules from; default: 0
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If stored rule data is invalid
*/
public function display_rules($language, $parent_id = 0)
{
Expand All @@ -250,8 +242,18 @@ public function display_rules($language, $parent_id = 0)
trigger_error($this->lang->lang('ACP_BOARDRULES_INVALID_LANGUAGE') . adm_back_link($this->u_action), E_USER_WARNING);
}

// Grab all the rules in the current user's language
$entities = $this->rule_operator->get_rules($language, $parent_id);
try
{
// Load both result sets before assigning template data, so a malformed
// stored rule cannot leave a partially rendered ACP page.
$entities = $this->rule_operator->get_rules($language, $parent_id);
$parent_entities = $this->rule_operator->get_rule_parents($language, $parent_id);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Initialize a variable to hold the right_id value
$last_right_id = 0;
Expand Down Expand Up @@ -282,11 +284,8 @@ public function display_rules($language, $parent_id = 0)
$last_right_id = $entity->get_right_id();
}

// Prepare rule breadcrumb path navigation
$entities = $this->rule_operator->get_rule_parents($language, $parent_id);

// Process each rule entity for breadcrumb display
foreach ($entities as $entity)
foreach ($parent_entities as $entity)
{
// Set output block vars for display in the template
$this->template->assign_block_vars('breadcrumb', array(
Expand Down Expand Up @@ -514,7 +513,6 @@ protected function get_ruleset_return_url($language, $return_to)
* @param int $parent_id Category to display rules from; default: 0
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If stored rule data is invalid
*/
public function add_rule($language, $parent_id = 0)
{
Expand All @@ -523,7 +521,7 @@ public function add_rule($language, $parent_id = 0)

// Initiate a rule entity
/* @var $entity \phpbb\boardrules\entity\rule */
$entity = $this->container->get('phpbb.boardrules.entity');
$entity = $this->rule_operator->create_rule();

// Collect the form data
$data = array(
Expand All @@ -538,7 +536,15 @@ public function add_rule($language, $parent_id = 0)
);

// Process the new rule
$this->add_edit_rule_data($entity, $data);
try
{
$this->add_edit_rule_data($entity, $data);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Set output vars for display in the template
$this->template->assign_vars(array(
Expand All @@ -555,14 +561,21 @@ public function add_rule($language, $parent_id = 0)
* @param int $rule_id The rule identifier to edit
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If stored rule data is invalid
*/
public function edit_rule($rule_id)
{
// Add form key
add_form_key('add_edit_rule');

$entity = $this->load_rule($rule_id);
try
{
$entity = $this->load_rule($rule_id);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Collect the form data
$data = array(
Expand All @@ -577,7 +590,15 @@ public function edit_rule($rule_id)
);

// Process the edited rule
$this->add_edit_rule_data($entity, $data);
try
{
$this->add_edit_rule_data($entity, $data);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Set output vars for display in the template
$this->template->assign_vars(array(
Expand Down Expand Up @@ -683,14 +704,7 @@ protected function add_edit_rule_data($entity, $data)
if ($entity->get_id())
{
// Save the edited rule entity to the database
try
{
$entity->save();
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
}
$entity = $this->rule_operator->save_rule($entity);

// Change rule parent
if (isset($data['rule_parent_id']) && ($entity->get_parent_id() !== (int) $data['rule_parent_id']))
Expand All @@ -699,11 +713,7 @@ protected function add_edit_rule_data($entity, $data)
{
$this->rule_operator->change_parent($entity->get_id(), $data['rule_parent_id']);
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
}
catch (\Exception $e)
catch (\InvalidArgumentException|\RuntimeException $e)
{
trigger_error($this->lang->lang($e->getMessage()) . adm_back_link($this->u_action), E_USER_WARNING);
}
Expand All @@ -719,10 +729,6 @@ protected function add_edit_rule_data($entity, $data)
{
$this->rule_operator->add_rule($entity, $data['rule_language'], $data['rule_parent_id']);
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
}
catch (\InvalidArgumentException|\RuntimeException $e)
{
trigger_error($this->lang->lang($e->getMessage()) . adm_back_link($this->u_action), E_USER_WARNING);
Expand Down Expand Up @@ -779,7 +785,15 @@ protected function add_edit_rule_data($entity, $data)
*/
public function delete_rule($rule_id)
{
$entity = $this->load_rule($rule_id);
try
{
$entity = $this->load_rule($rule_id);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Use a confirmation box routine when deleting a rule
if (confirm_box(true))
Expand All @@ -789,13 +803,15 @@ public function delete_rule($rule_id)
{
$this->rule_operator->delete_rule($rule_id);
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
catch (\phpbb\boardrules\exception\base $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
$this->display_rule_error($e);
return;
}
catch (\Exception $e)
{
trigger_error($this->lang->lang($e->getMessage()) . adm_back_link($this->u_action), E_USER_WARNING);
return;
}

// Show user confirmation of the deleted rule and provide link back to the previous page
Expand Down Expand Up @@ -842,13 +858,15 @@ public function move_rule($rule_id, $direction, $amount = 1)
{
$moved = $this->rule_operator->move($rule_id, $direction, $amount);
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
catch (\phpbb\boardrules\exception\base $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
$this->display_rule_error($e);
return;
}
catch (\Exception $e)
{
trigger_error($this->lang->lang($e->getMessage()) . adm_back_link($this->u_action), E_USER_WARNING);
return;
}

// Send a JSON response if an AJAX request was used
Expand All @@ -858,7 +876,15 @@ public function move_rule($rule_id, $direction, $amount = 1)
$json_response->send(array('success' => $moved));
}

$entity = $this->load_rule($rule_id);
try
{
$entity = $this->load_rule($rule_id);
}
catch (\phpbb\boardrules\exception\base $e)
{
$this->display_rule_error($e);
return;
}

// Use a redirect to reload the current page
redirect("{$this->u_action}&language={$entity->get_language()}&parent_id={$entity->get_parent_id()}");
Expand Down Expand Up @@ -915,21 +941,26 @@ public function set_page_url($u_action)
}

/**
* Load a rule or display a recoverable ACP error when it no longer exists.
* Load a rule.
*
* @param int $rule_id Rule identifier
* @return \phpbb\boardrules\entity\rule_interface
* @throws \phpbb\boardrules\exception\base If the rule is missing or stored data is invalid
*/
protected function load_rule($rule_id)
{
try
{
return $this->container->get('phpbb.boardrules.entity')->load($rule_id);
}
catch (\phpbb\boardrules\exception\out_of_bounds $e)
{
trigger_error($e->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
}
return $this->rule_operator->get_rule($rule_id);
}

/**
* Display a translated entity or operator failure in the ACP.
*
* @param \phpbb\boardrules\exception\base $exception
* @return void
*/
protected function display_rule_error(\phpbb\boardrules\exception\base $exception)
{
trigger_error($exception->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING);
}

/**
Expand Down
7 changes: 1 addition & 6 deletions controller/admin_interface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/**
* Interface for our admin controller
*
* This describes all of the methods we'll use for the admin front-end of this extension
* This describes all the methods we'll use for the admin front-end of this extension
*/
interface admin_interface
{
Expand Down Expand Up @@ -59,7 +59,6 @@ public function set_ruleset_published($language, $published, $return_to = '');
* @param int $parent_id Category to display rules from; default: 0
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If stored rule data is invalid
*/
public function display_rules($language, $parent_id = 0);

Expand All @@ -78,7 +77,6 @@ public function save_ruleset_intro($language);
* @param int $parent_id Category to display rules from; default: 0
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If stored rule data is invalid
*/
public function add_rule($language, $parent_id = 0);

Expand All @@ -88,7 +86,6 @@ public function add_rule($language, $parent_id = 0);
* @param int $rule_id The rule identifier to edit
* @return void
* @access public
* @throws \phpbb\boardrules\exception\base If the rule does not exist or stored rule data is invalid
*/
public function edit_rule($rule_id);

Expand All @@ -98,7 +95,6 @@ public function edit_rule($rule_id);
* @param int $rule_id The rule identifier to delete
* @return void
* @access public
* @throws \phpbb\boardrules\exception\out_of_bounds If the rule does not exist
*/
public function delete_rule($rule_id);

Expand All @@ -110,7 +106,6 @@ public function delete_rule($rule_id);
* @param int $amount The number of places to move the rule
* @return void
* @access public
* @throws \phpbb\boardrules\exception\out_of_bounds If the rule does not exist after moving
*/
public function move_rule($rule_id, $direction, $amount = 1);

Expand Down
Loading
Loading