fix: align service pagination and endpoint behavior#3
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThis PR enhances the WordPress SDK with optional parameter defaults, refines service implementations for WordPress endpoint handling, and adds extensive test coverage validating data model construction, pagination behavior, and endpoint path generation across multiple services. ChangesSDK Data Models, Service Enhancements, and Test Coverage
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Services/BlockRendererService.php (1)
17-17: ⚡ Quick winReplace inline context literal with a class constant.
Line 17 hardcodes
'edit'; extract it to a named constant so the value is centralized and less error-prone.As per coding guidelines, "Avoid hidden domain strings when enums or constants are clearer".Suggested diff
class BlockRendererService extends RawEndpointService { + private const CONTEXT_EDIT = 'edit'; + /** * `@param` array<string, mixed> $attributes * `@return` array<mixed> */ public function render(string $name, array $attributes = [], ?int $postId = null): array { - $query = ['context' => 'edit']; + $query = ['context' => self::CONTEXT_EDIT]; if ($attributes !== []) { $query['attributes'] = $attributes; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Services/BlockRendererService.php` at line 17, Replace the hardcoded context string by adding a class constant on BlockRendererService (e.g., private const CONTEXT_EDIT = 'edit') and update the $query assignment to use that constant (self::CONTEXT_EDIT) instead of the literal; also search for any other occurrences of the literal 'edit' in BlockRendererService and switch them to the same constant so the domain value is centralized and less error-prone.tests/Unit/Services/RawEndpointServiceTest.php (1)
19-22: ⚡ Quick winRemove this non-invariant docblock from the anonymous test helper.
This comment is type-only and does not capture why, tradeoffs, or invariants.
As per coding guidelines, "Keep comments for why, tradeoffs, or invariants only".Suggested diff
- /** - * `@param` array<string, mixed> $payload - * `@return` array<mixed> - */ public function exposedPutRaw(string $path, array $payload): array { return $this->putRaw($path, $payload); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Services/RawEndpointServiceTest.php` around lines 19 - 22, Remove the non-informative docblock on the anonymous test helper that only repeats types (the block containing "`@param` array<string, mixed> $payload" and "`@return` array<mixed>"); delete this comment so the test helper remains uncluttered (rely on actual type hints or PHPStan/phpdoc where meaningful), leaving the anonymous function itself untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Services/PluginsService.php`:
- Around line 52-57: pluginPath currently builds a path from explode('/'...)
without filtering empty segments which allows inputs like '' or 'a//b' to
produce malformed endpoints; update pluginPath to first
explode(trim($plugin,'/')) then array_filter the segments to remove empty
strings, throw an InvalidArgumentException (or other appropriate exception) if
the resulting segments array is empty (fail fast), and finally map each
remaining segment through $this->segment(...) and implode with '/' to produce
the normalized path (refer to function pluginPath and method segment).
---
Nitpick comments:
In `@src/Services/BlockRendererService.php`:
- Line 17: Replace the hardcoded context string by adding a class constant on
BlockRendererService (e.g., private const CONTEXT_EDIT = 'edit') and update the
$query assignment to use that constant (self::CONTEXT_EDIT) instead of the
literal; also search for any other occurrences of the literal 'edit' in
BlockRendererService and switch them to the same constant so the domain value is
centralized and less error-prone.
In `@tests/Unit/Services/RawEndpointServiceTest.php`:
- Around line 19-22: Remove the non-informative docblock on the anonymous test
helper that only repeats types (the block containing "`@param` array<string,
mixed> $payload" and "`@return` array<mixed>"); delete this comment so the test
helper remains uncluttered (rely on actual type hints or PHPStan/phpdoc where
meaningful), leaving the anonymous function itself untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71c6b530-c89c-4b90-9d15-db4496296c82
📒 Files selected for processing (28)
src/Data/Page.phpsrc/Data/Status.phpsrc/Http/AbstractService.phpsrc/Services/BlockRendererService.phpsrc/Services/PluginsService.phptests/Unit/Data/DataModelsTest.phptests/Unit/Data/Query/ListQueryTest.phptests/Unit/Services/AbstractTermServiceTest.phptests/Unit/Services/BlockRendererServiceTest.phptests/Unit/Services/BlockTypesServiceTest.phptests/Unit/Services/BlocksServiceTest.phptests/Unit/Services/CommentsServiceTest.phptests/Unit/Services/EndpointPathServicesTest.phptests/Unit/Services/GlobalStylesServiceTest.phptests/Unit/Services/MediaServiceTest.phptests/Unit/Services/MenuLocationsServiceTest.phptests/Unit/Services/NavMenusServiceTest.phptests/Unit/Services/PagesServiceTest.phptests/Unit/Services/PluginsServiceTest.phptests/Unit/Services/RawEndpointServiceTest.phptests/Unit/Services/SchemaServicesTest.phptests/Unit/Services/SearchServiceTest.phptests/Unit/Services/SidebarsServiceTest.phptests/Unit/Services/SiteHealthServiceTest.phptests/Unit/Services/TemplatesServiceTest.phptests/Unit/Services/ThemesServiceTest.phptests/Unit/Services/UsersServiceTest.phptests/Unit/Services/WidgetTypesServiceTest.php
💤 Files with no reviewable changes (1)
- src/Http/AbstractService.php
Comment on lines
+52
to
+57
| private function pluginPath(string $plugin): string | ||
| { | ||
| return implode('/', array_map( | ||
| fn (string $segment): string => $this->segment($segment), | ||
| explode('/', trim($plugin, '/')) | ||
| )); |
There was a problem hiding this comment.
Validate and normalize empty plugin segments before URI construction.
Lines 52-57 currently allow '', '/', or double-slash input ('a//b'), which can generate malformed plugin endpoints. Filter empty segments and fail fast when no valid segment remains.
Suggested diff
private function pluginPath(string $plugin): string
{
- return implode('/', array_map(
+ $segments = array_values(array_filter(
+ explode('/', trim($plugin, '/')),
+ static fn (string $segment): bool => $segment !== ''
+ ));
+
+ if ($segments === []) {
+ throw new \InvalidArgumentException('Plugin path must not be empty.');
+ }
+
+ return implode('/', array_map(
fn (string $segment): string => $this->segment($segment),
- explode('/', trim($plugin, '/'))
+ $segments
));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private function pluginPath(string $plugin): string | |
| { | |
| return implode('/', array_map( | |
| fn (string $segment): string => $this->segment($segment), | |
| explode('/', trim($plugin, '/')) | |
| )); | |
| private function pluginPath(string $plugin): string | |
| { | |
| $segments = array_values(array_filter( | |
| explode('/', trim($plugin, '/')), | |
| static fn (string $segment): bool => $segment !== '' | |
| )); | |
| if ($segments === []) { | |
| throw new \InvalidArgumentException('Plugin path must not be empty.'); | |
| } | |
| return implode('/', array_map( | |
| fn (string $segment): string => $this->segment($segment), | |
| $segments | |
| )); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Services/PluginsService.php` around lines 52 - 57, pluginPath currently
builds a path from explode('/'...) without filtering empty segments which allows
inputs like '' or 'a//b' to produce malformed endpoints; update pluginPath to
first explode(trim($plugin,'/')) then array_filter the segments to remove empty
strings, throw an InvalidArgumentException (or other appropriate exception) if
the resulting segments array is empty (fail fast), and finally map each
remaining segment through $this->segment(...) and implode with '/' to produce
the normalized path (refer to function pluginPath and method segment).
There was a problem hiding this comment.
Pull request overview
This PR aligns WordPress service endpoint/path handling and pagination helper behavior across multiple services, and expands unit test coverage to lock in the expected request URIs/options and DTO/query mapping.
Changes:
- Adjusted service behaviors for endpoint path construction (notably plugin path handling) and block rendering context defaults.
- Expanded/added unit tests to validate endpoint paths, request options, and auto-pagination helpers across many services.
- Improved DTO ergonomics for WordPress-shaped responses (e.g.,
Statusdefaults,Pagepost-field defaults) and added query mapping tests.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Services/WidgetTypesServiceTest.php | Adds coverage for widget-types list/get request paths and options. |
| tests/Unit/Services/UsersServiceTest.php | Adds coverage for auto-pagination helpers behavior (all, cursor, each). |
| tests/Unit/Services/ThemesServiceTest.php | Adds coverage for themes list path and query mapping. |
| tests/Unit/Services/TemplatesServiceTest.php | Adds coverage for templates list/create paths and request bodies. |
| tests/Unit/Services/SiteHealthServiceTest.php | Adds coverage for additional site health check helper methods and paths. |
| tests/Unit/Services/SidebarsServiceTest.php | Adds coverage for sidebars list/get paths and query usage. |
| tests/Unit/Services/SearchServiceTest.php | Adds coverage for auto-pagination helpers on search endpoint. |
| tests/Unit/Services/SchemaServicesTest.php | Adds auto-pagination coverage for schema-backed services and captures last request options. |
| tests/Unit/Services/RawEndpointServiceTest.php | New test validating putRaw request body construction and request recording. |
| tests/Unit/Services/PluginsServiceTest.php | Updates/expands tests to reflect new plugin path handling and CRUD request building. |
| tests/Unit/Services/PagesServiceTest.php | Adds coverage for auto-pagination helpers on pages endpoint. |
| tests/Unit/Services/NavMenusServiceTest.php | Adds coverage for menu get/delete paths and force query. |
| tests/Unit/Services/MenuLocationsServiceTest.php | Adds coverage for menu locations list path behavior. |
| tests/Unit/Services/MediaServiceTest.php | Adds coverage for auto-pagination helpers on media endpoint. |
| tests/Unit/Services/GlobalStylesServiceTest.php | Adds coverage for global styles CRUD paths and request bodies. |
| tests/Unit/Services/EndpointPathServicesTest.php | New consolidated path regression tests across multiple endpoint services (also introduces local test helpers). |
| tests/Unit/Services/CommentsServiceTest.php | Adds coverage for auto-pagination helpers on comments endpoint. |
| tests/Unit/Services/BlockTypesServiceTest.php | Adds coverage for block-types list path variants and query mapping. |
| tests/Unit/Services/BlocksServiceTest.php | Adds coverage for blocks CRUD path building and request body mapping. |
| tests/Unit/Services/BlockRendererServiceTest.php | Updates assertions to require context=edit in block renderer queries. |
| tests/Unit/Data/Query/ListQueryTest.php | Adds tests for media/terms/users list query-to-querystring mapping and null filtering. |
| tests/Unit/Data/DataModelsTest.php | Adds DTO constructor coverage for Page, Status, and other WP response shapes. |
| src/Services/PluginsService.php | Changes plugin path construction to encode segments while keeping / separators. |
| src/Services/BlockRendererService.php | Defaults render requests to context=edit. |
| src/Http/AbstractService.php | Removes extraneous inline comments (no functional change in diff shown). |
| src/Data/Status.php | Provides default false values for optional WordPress boolean fields. |
| src/Data/Page.php | Adds Page constructor supplying defaults for post-only fields not present in typical page responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+259
to
+263
| class NullHttpClient implements HttpClientInterface | ||
| { | ||
| public function get(string $uri, array $options = []): ResponseWrapperInterface | ||
| { | ||
| throw new \RuntimeException('Not used'); |
Comment on lines
+142
to
+146
| trait RecordsServiceRequests | ||
| { | ||
| public ?string $lastMethod = null; | ||
| public ?string $lastUri = null; | ||
| public array $lastOptions = []; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Aligned service pagination and endpoint behavior.
Why
Consistency and correctness in service responses.
How
Updated pagination logic and endpoint handlers.
Files/areas touched
Service classes and endpoints.
Validation commands and results
composer test, composer lint, composer security, composer quality all passed.
Dependency on other PRs
None.
Risk/impact
Low-impact fix for existing services.
Summary by CodeRabbit
Bug Fixes
Chores