Skip to content

Conversation

@davebarnwell
Copy link
Owner

Summary

  • support camelCase dynamic finder/counter names (findByName, countByName, etc.)
  • resolve field names against actual column names and snake_case
  • add PHPUnit coverage and update docs/annotations

Testing

  • not run (not requested)

Copilot AI review requested due to automatic review settings January 30, 2026 22:56
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.

Reviewed commit: 0dea91ba31

ℹ️ 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 +596 to +599
protected static function camelToSnake($fieldname)
{
$snake = preg_replace('/(?<!^)[A-Z]/', '_$0', $fieldname);
return strtolower($snake ?? $fieldname);

Choose a reason for hiding this comment

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

P2 Badge Handle acronym segments when converting camelCase to snake_case

The new camelToSnake helper splits before every capital letter, so camelCase names with acronym runs (e.g., findByProductID or findByURLValue) become product_i_d/u_r_l_value. resolveFieldName then fails to match the actual snake_case column (e.g., product_id or url_value) and falls back to the original productID, which is quoted and queried as a non-existent column. This will throw SQL errors or return no rows whenever a model has acronym-style column names and users rely on the new camelCase dynamic finders. Consider a regex that collapses consecutive capitals into a single word boundary.

Useful? React with 👍 / 👎.

@davebarnwell davebarnwell merged commit db1ae8f into master Jan 30, 2026
10 checks passed
Copy link

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 extends the model’s dynamic finder/counter API to support CamelCase method names and more flexible field-name resolution, and adds corresponding annotations, tests, and docs. It also refactors dynamic count logic to reuse shared helpers.

Changes:

  • Add CamelCase dynamic finder/counter support in Model::__callStatic (e.g., findByName, findOneByName, firstByName, lastByName, countByName) and introduce resolveFieldName, camelToSnake, and countByField helpers to resolve method segments to actual column names, including snake_case.
  • Update the App\Model\Category test model annotations and add CategoryTest::testDynamicFindersCamelCase to exercise the new CamelCase dynamic methods.
  • Extend the README dynamic finder section with documentation and examples for CamelCase alternatives.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Model/Model.php Adds CamelCase dynamic method handling plus shared helpers for resolving field names and counting by field, centralizing and extending dynamic finder behavior.
tests/Model/CategoryTest.php Introduces a new test that verifies the CamelCase dynamic finders and counter for the name field using the Category model.
test-src/Model/Category.php Updates model docblock annotations to declare the new CamelCase dynamic finder/counter methods for static analysis and IDE support.
README.md Documents CamelCase alternatives for dynamic finders, showing example usage alongside the existing underscore-based methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +566 to +600
protected static function resolveFieldName($fieldname)
{
$fieldnames = static::getFieldnames();
if (in_array($fieldname, $fieldnames, true)) {
return $fieldname;
}
foreach ($fieldnames as $field) {
if (strcasecmp($field, $fieldname) === 0) {
return $field;
}
}
$snake = static::camelToSnake($fieldname);
if (in_array($snake, $fieldnames, true)) {
return $snake;
}
foreach ($fieldnames as $field) {
if (strcasecmp($field, $snake) === 0) {
return $field;
}
}
return $fieldname;
}

/**
* Convert CamelCase or mixedCase to snake_case.
*
* @param string $fieldname
*
* @return string
*/
protected static function camelToSnake($fieldname)
{
$snake = preg_replace('/(?<!^)[A-Z]/', '_$0', $fieldname);
return strtolower($snake ?? $fieldname);
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The new resolveFieldName/camelToSnake logic that maps CamelCase method segments to snake_case column names (e.g. findByCreatedAt -> created_at) is not covered by tests; the current testDynamicFindersCamelCase only exercises simple case-insensitive matching for name and never hits the camel-to-snake path. Please add test coverage for at least one snake_case column (such as created_at) using the new CamelCase dynamic methods so this behavior is verified and guarded against regressions.

Copilot uses AI. Check for mistakes.
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