Skip to content

refactor: deduplicate selected-item bulk action dispatch#279

Open
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:refactor/bulk-action-dispatch
Open

refactor: deduplicate selected-item bulk action dispatch#279
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:refactor/bulk-action-dispatch

Conversation

@somethingwithproof
Copy link
Contributor

Summary

Fixes #276

Validation

  • php -l functions.php
  • php -l syslog_alerts.php
  • php -l syslog_reports.php
  • php -l syslog_removal.php
  • php -l tests/regression/issue276_bulk_action_dispatch_helper_test.php
  • php tests/regression/issue276_bulk_action_dispatch_helper_test.php

Copilot AI review requested due to automatic review settings March 7, 2026 02:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Syslog plugin’s rule pages (alerts/reports/removal) to remove duplicated “selected items” bulk-action dispatch logic by introducing a shared helper in functions.php, while keeping per-page action maps local to preserve behavior.

Changes:

  • Add syslog_apply_selected_items_action() helper to centralize selected-item action dispatch and export session storage.
  • Update form_actions() handlers in alerts/reports/removal pages to use the shared helper.
  • Add a small regression test to assert the helper exists and is referenced by all three pages; update changelog for issue #276.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
functions.php Introduces shared selected-item bulk-action dispatch helper (including export session handling).
syslog_alerts.php Refactors bulk action dispatch in form_actions() to call the shared helper.
syslog_reports.php Refactors bulk action dispatch in form_actions() to call the shared helper.
syslog_removal.php Refactors bulk action dispatch in form_actions() to call the shared helper.
tests/regression/issue276_bulk_action_dispatch_helper_test.php Adds regression coverage to detect removal/non-usage of the shared helper (string-based).
CHANGELOG.md Adds issue #276 entry under develop.

@somethingwithproof somethingwithproof force-pushed the refactor/bulk-action-dispatch branch 3 times, most recently from 2278517 to 791fce6 Compare March 10, 2026 20:30
somethingwithproof added a commit to somethingwithproof/plugin_syslog that referenced this pull request Mar 10, 2026
Refs Cacti#279

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the refactor/bulk-action-dispatch branch from 791fce6 to d4f5d0c Compare March 10, 2026 20:55
somethingwithproof added a commit to somethingwithproof/plugin_syslog that referenced this pull request Mar 15, 2026
Refs Cacti#279

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the refactor/bulk-action-dispatch branch from d4f5d0c to 1a76e1d Compare March 15, 2026 05:17
Refs Cacti#279

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copy link
Member

@TheWitness TheWitness left a comment

Choose a reason for hiding this comment

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

More merge conflicts.

@somethingwithproof somethingwithproof force-pushed the refactor/bulk-action-dispatch branch from 1a76e1d to 72d8389 Compare March 16, 2026 20:59
@TheWitness
Copy link
Member

So, let's do the following in this pull @somethingwithproof:

  1. Refix functions.php to remove $uniqueID in the query
  2. Reorder the workflow to put Lint, CS-Fixer, and PHPStan before right after the PHP is installed and the plugin is ready
  3. Switch from echo to print. At some point, we will have to validate that.
  4. Make sure the pre-commit hook is successful by running composer run-script plugins/syslog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: deduplicate bulk action item handling across alerts/reports/removal pages

3 participants