FIX: return empty list instead of 404 for no records https://github.com/Crocoblock/issues-tracker/issues/20076

[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: low

Review

Summary

• This PR changes the fetch-records REST endpoint (modules/form-record/rest-endpoints/fetch-records-page-endpoint.php) to return a 200 response with an empty list instead of returning a 404 when no records are found.

What I like

• Returning an empty list with a total is a better UX for API consumers (they get consistent response shape and can rely on total/pagination), and fixes the issue linked in the PR.

Things to consider / suggestions

1. Explicit HTTP status

• Currently WP_REST_Response defaults to 200, but it's clearer if the code explicitly sets the status to 200 when returning a successful payload. Consider new \WP_REST_Response( $payload, 200 ) to make intent explicit.

2. Tests

• Add a unit/integration test covering:
  • No records at all -> response status 200, 'list' is empty array, 'total' is 0.
  • Page with no items but overall total > 0 (e.g. requesting a page beyond available pages) -> response status 200, 'list' is empty array, 'total' > 0. This ensures the client can distinguish between empty page vs no data.

3. Backward compatibility

• This is a behavioural change in the HTTP status code for the endpoint (404 -> 200). While this is the intended fix, document the change in the changelog and note it for integrators. Any clients that depended on 404 should be updated.

4. Performance / count cost

• Record_View_Count::count( $args ) likely runs a separate query to compute the total. For large datasets this can be expensive. Consider caching/count optimizations if this endpoint is called frequently for large tables. If such optimisations exist elsewhere in the codebase, it's OK as-is; otherwise add a TODO or consider batching/caching.

5. Security

• The change itself only affects response shape. Ensure the rest of run_callback still performs necessary capability checks and nonce validation. This PR does not appear to remove any such checks, but please confirm run_callback validates input and checks permissions earlier in the method.

6. Minor code-style

• Current code uses array() which matches surrounding code; fine to keep. Optionally explicitly set response status (see point 1).

Files / functions reviewed

• modules/form-record/rest-endpoints/fetch-records-page-endpoint.php
  • run_callback()
  • $view->get_raw_list(), $view->get_prepared_list(), Record_View_Count::count()

Conclusion

• The change is appropriate and improves API ergonomics. Please add tests, set explicit 200 status for clarity, and document the behavior change for integrators that may depend on the previous 404.

Suggested changelog entry

- FIX: return empty list instead of 404 for no records (form-record REST endpoint)