Skip to content

Conversation

@erseco
Copy link
Collaborator

@erseco erseco commented Dec 1, 2025

This pull request introduces several improvements focused on error handling, code organization, and testability in the Documentate plugin, especially in the admin helper and WP-CLI integration. The main changes include refactoring admin helper methods for better error management, restructuring WP-CLI command registration for improved testability, and enhancing the coverage summary reporting. Below are the most significant updates grouped by theme:

Admin Helper Error Handling & Refactoring:

  • Refactored PDF preview and stream handlers in class-documentate-admin-helper.php to use dedicated response preparation methods and return WP_Error objects on failure instead of immediately calling wp_die(). This centralizes error handling and improves maintainability. [1] [2] [3] [4]
  • Added helper methods for AJAX document generation, separating nonce verification, permission checks, and error building, making the logic clearer and more robust. [1] [2] [3]

Converter Page Rendering Improvements:

  • Refactored the converter page rendering in class-documentate-admin-helper.php to separate header preparation and template path resolution into their own methods, improving readability and allowing easier testing. [1] [2]

WP-CLI Command Structure & Testability:

  • Changed Documentate_WPCLI to no longer extend WP_CLI_Command directly, and moved command registration inside a conditional block, allowing easier mocking and testing of CLI methods. Also, abstracted WP-CLI output and confirmation methods for test overrides. [1] [2] [3]

Test Coverage Reporting:

  • Updated the Makefile to use a new AWK script (scripts/coverage-summary.awk) for coverage summary reporting, providing more readable and targeted output. Added the AWK script to the repository. [1] [2]

Packaging & Export Ignore Updates:

  • Added several new paths to the .gitattributes file to be excluded from export, helping to reduce package size and prevent unnecessary files from being distributed. [1] [2]

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 900 to +980
public function ajax_generate_document() {
// Verify nonce first before accessing any other POST data.
if ( ! isset( $_POST['_wpnonce'] ) || ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['_wpnonce'] ) ), 'documentate_generate' ) ) {
wp_send_json_error( array( 'message' => __( 'Security check failed.', 'documentate' ) ) );

Choose a reason for hiding this comment

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

P1 Badge Align AJAX generate nonce with issued nonce action

The new nonce check in ajax_generate_document() now verifies against the fixed action 'documentate_generate', but the configuration sent to the front‑end still issues nonces using the post‑specific action 'documentate_generate_' . $post_id (see build_actions_script_config()). Any document generation request made from the UI will therefore fail nonce verification and return the “Security check failed.” JSON error, blocking downloads/previews for all posts. The handler should validate against the same post‑scoped action it generates.

Useful? React with 👍 / 👎.

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 42.71845% with 59 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
includes/class-documentate-admin-helper.php 43.75% 45 Missing ⚠️
includes/class-documentate-wpcli.php 39.13% 14 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

2 participants