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

Fix contao:user:list with empty database #4074

Merged
merged 12 commits into from Feb 9, 2022
Merged

Fix contao:user:list with empty database #4074

merged 12 commits into from Feb 9, 2022

Conversation

AlexanderWillner
Copy link
Contributor

When executing contao-console contao:user:list on an empty database we'd receive the following error otherwise:
Return value of Contao\CoreBundle\Command\UserListCommand::getUsers() must be an instance of Contao\Model\Collection, null returned.

With this change instead we receive a nice warning: ! [NOTE] No accounts found.

When executing 'contao-console contao:user:list' on an empty database
we'd receive the following error otherwise:
'Return value of Contao\CoreBundle\Command\UserListCommand::getUsers() must be an instance of Contao\Model\Collection, null returned'
@aschempp
Copy link
Member

aschempp commented Feb 5, 2022

nice catch! I think we should use an array instead of returning a collection, then this problem would have happened in the first place. This also does not fix the problem when checking for admin only accounts, as that might still return null.

@AlexanderWillner
Copy link
Contributor Author

nice catch! I think we should use an array instead of returning a collection, then this problem would have happened in the first place.

I've refactored the code in d2d4236 - have you had something like this in mind?

This also does not fix the problem when checking for admin only accounts, as that might still return null.

right - also fixed in d2d4236

@leofeyer leofeyer added the bug label Feb 6, 2022
@leofeyer leofeyer added this to the 4.13 milestone Feb 6, 2022
@leofeyer
Copy link
Member

leofeyer commented Feb 6, 2022

Why not allow null as return type?

private function getUsers(bool $onlyAdmins = false): ?Collection

@AlexanderWillner
Copy link
Contributor Author

Why not allow null as return type?

private function getUsers(bool $onlyAdmins = false): ?Collection

That's an alternative. In this case we'd need to change line 69 from

if (0 === $users->count()) {

to something like

if (! $users || 0 === $users->count()) {

as well. Otherwise we get a the error Call to a member function count() on null.

@leofeyer leofeyer changed the title Fixing 'contao-console contao:user:list' on empty database. Fixing contao:user:list with empty database Feb 8, 2022
@leofeyer leofeyer changed the title Fixing contao:user:list with empty database Fix contao:user:list with empty database Feb 8, 2022
Copy link
Member

@aschempp aschempp left a comment

Choose a reason for hiding this comment

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

Thanks @AlexanderWillner and sorry for having to refactor this twice 😅

@leofeyer leofeyer merged commit 513359e into contao:4.13 Feb 9, 2022
@leofeyer
Copy link
Member

leofeyer commented Feb 9, 2022

Thank you @AlexanderWillner.

@AlexanderWillner
Copy link
Contributor Author

Thanks @AlexanderWillner and sorry for having to refactor this twice 😅

refactoring is fun ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants