Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force int cast on ModelsLoader count #2646

Merged
merged 6 commits into from
Jan 8, 2025
Merged

Conversation

robsontenorio
Copy link
Contributor

@robsontenorio robsontenorio commented Dec 28, 2024

When dealing with legacy databases the count for this related model returns a string like "88", not 88.

comments: [Comment] @hasMany(type: PAGINATOR)

After upgrading to PHP 8.4 this does not work anymore. And it throws a error

"Nuwave\Lighthouse\Execution\ModelsLoader\CountModelsLoader::extractCount(): Return value must be of type int, string returned"

Changes

Always cast to integer. Because, this count must be an integer, always.

Breaking changes

No.

When dealing with **legacy databases**  the count for this related model returns a string like `"88"`, not `88`.

```
comments: [Comment] @hasmany(type: PAGINATOR)
```

After upgrading to PHP 8.4 this does not work anymore. And it throws a error 

_"Nuwave\Lighthouse\Execution\ModelsLoader\CountModelsLoader::extractCount(): Return value must be of type int, string returned"_


**Changes**

Always cast to integer. Because, this count must be an integer, always.

**Breaking changes**

No.
@robsontenorio
Copy link
Contributor Author

@spawnia your suggested approach didn’t work.

I also fixed the quote issue, but no luck.

@spawnia
Copy link
Collaborator

spawnia commented Jan 3, 2025

How is it not working? It is still casting to int.

@robsontenorio
Copy link
Contributor Author

robsontenorio commented Jan 3, 2025

@spawnia the pipeline is failing :(

it was passing at my first commit. But not after yours.

@spawnia
Copy link
Collaborator

spawnia commented Jan 7, 2025

I added the missing import with robsontenorio@2484c84, GitHub seems stuck at the moment.

@robsontenorio robsontenorio requested a review from spawnia January 8, 2025 00:15
@robsontenorio
Copy link
Contributor Author

@spawnia i forced an dummy commit but I don’t get what is wrong with GitHub actions. Now phpstan complains about other things.

@spawnia spawnia merged commit cb5b81a into nuwave:master Jan 8, 2025
@spawnia
Copy link
Collaborator

spawnia commented Jan 8, 2025

Released with https://github.com/nuwave/lighthouse/releases/tag/v6.47.1, thank you @robsontenorio

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