Skip to content

Conversation

colinmurphy
Copy link
Member

@colinmurphy colinmurphy commented Aug 20, 2025

Description

This PR focuses on creating the admin functionality around displaying and saving configuration.

The initial PR creates configuration for Basic Configuration but we expect to add more tabs as we progress the logging plugin.

This PR integrates the settings into the plugin's core functionality.

Related Issue

#353

Dependant PRs

Type of Change

  • ✅ Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactoring (no functional changes)
  • 📄 Example update (no functional changes)
  • 📝 Documentation update
  • 🔍 Performance improvement
  • 🧪 Test update

How Has This Been Tested?

Screenshots

Screenshot 2025-08-20 at 17 56 54 Screenshot 2025-08-20 at 17 57 25

Checklist

  • I have read the CONTRIBUTING document
  • My code follows the project's coding standards
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • Any dependent changes have been highlighted, merged or published

Use Cases for Testing

To ensure the new functionality works as expected, you should test the following scenarios.


1. Global Enable/Disable

  • Test 1 (Disabled): Go to the "Basic Configuration" tab, uncheck the "Enabled" checkbox, and save. Perform a GraphQL query. Expected result: No log entries are created for the request.
  • Test 2 (Enabled): Check the "Enabled" checkbox and save. Perform another GraphQL query. Expected result: A log entry is created.

2. IP Restrictions

  • Test 1 (Allowed IP): Enter your current IP address (e.g., 192.168.1.1) in the "IP Restrictions" field and save. Perform a GraphQL query. Expected result: A log entry is created.
  • Test 2 (Denied IP): Change the IP address in the field to a different one (e.g., 10.0.0.1) and save. Perform a GraphQL query from your machine. Expected result: No log entry is created.

3. Admin User Logging

  • Test 1 (Logged-in Admin): Log in as an administrator. Check the "Log only for admin users" box and save. Perform a GraphQL query. Expected result: A log entry is created.
  • Test 2 (Logged-out User): Log out of WordPress or use an incognito window. Perform a GraphQL query. Expected result: No log entry is created.
  • Test 3 (Disabled): Uncheck the "Log only for admin users" box and save. Perform a GraphQL query as a logged-out user. Expected result: A log entry is created.

4. Data Sampling

  • Test 1 (100% Sampling): Set the "Data Sampling Rate" to 100%. Perform several GraphQL queries. Expected result: A log entry is created for every single request.
  • Test 2 (10% Sampling): Set the "Data Sampling Rate" to 10%. Perform many GraphQL queries (e.g., 20 requests). Expected result: A small number of log entries are created, roughly 2 out of 20, but the exact number may vary due to the random nature of sampling.

5. Event Log Selection

  • Test 1 (All Events): Select all events in the "Log Points" field and save. Perform a single GraphQL query. Expected result: You should see multiple logs, one for each hook: "Pre Request", "Before Query Execution", and "Before Response Returned".
  • Test 2 (Single Event): Select only one event, for example, "Pre Request", and save. Perform a single GraphQL query. Expected result: Only one log entry is created for that specific event.

Refactored Select_Field sanitization to handle non-string values and filter out invalid options. Removed redundant is_admin() check in Settings_Page::init. Added comprehensive unit tests for Settings_Page to verify initialization, hook registration, tab selection, and asset loading behaviors.
Added detailed admin settings documentation in docs/admin.md and updated README.md with references to admin architecture and extension hooks. Modified Basic_Configuration_Tab to allow fields to be filtered via 'wpgraphql_logging_basic_configuration_fields', enabling easier extension of admin settings.
Copy link

changeset-bot bot commented Aug 20, 2025

⚠️ No Changeset found

Latest commit: 69c15b0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

github-actions bot commented Aug 20, 2025

ℹ️ Download the latest wpgraphql-logging plugin zip from this PR
(See the 'Artifacts' section at the bottom)

@colinmurphy colinmurphy marked this pull request as ready for review August 20, 2025 16:58
@colinmurphy colinmurphy requested a review from a team as a code owner August 20, 2025 16:58
Adds --ssl-mode=DISABLED to mysql and mysqladmin commands in install-test-env.sh to prevent SSL connection issues during database setup.
@josephfusco
Copy link
Member

@colinmurphy The latest zip is failing to activate in a new WordPress install. It could just be a plugin error preventing activation. I tried an earlier version of the zip but the same thing happened.

There might be something going on with just the activation step.

@colinmurphy
Copy link
Member Author

Thanks @josephfusco

I will test this in the next PR and thanks for the heads up.

Copy link
Member

@josephfusco josephfusco left a comment

Choose a reason for hiding this comment

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

Nice work on this @colinmurphy!

I found a small PHP compatibility issue if support for php 7. Once that's updated/confirmed not an issue this looks ready to me!

*/
public function load_scripts_styles( string $hook_suffix ): void {
// Only load on our settings page.
if ( ! str_contains( $hook_suffix, self::PLUGIN_MENU_SLUG ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

str_contains() is 8.0, consider using strpos() for 7.4 if support is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @josephfusco

The plugin intentionally is for PHP 8.1 and above but maybe we should put a notice for PHP 7 users.

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.

3 participants