Skip to content

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Jun 20, 2025

Some errors reported by PHPStan level 7 is related to the fact that

$actionsDto->getActions()->all()

might be non-ok if getActions return an array, since the return type is

@return ActionCollection|array<string,array<string,ActionDto>>

The issue is this method does two things

  • returning everything
  • or returning the value of a single page name.

A solution could be to split the method in two: getPageActions and getActionList (naming is challengeable).
Also getPageActions can take the pageName as args rather than relying on a private property since the page is accessible with

$this->adminContextProvider->getContext()->getCrud()->getCurrentPage()

(and you might want to know the actions of another page)

Then I deprecated setPageName (but I cannot trigger a deprecation since it has to be used in easyAdmin code for BC)

This PR should solve issue like #6531 or at least helps for debug.

@VincentLanglet VincentLanglet marked this pull request as ready for review June 20, 2025 11:41
@VincentLanglet
Copy link
Contributor Author

Do you mind taking a look @javiereguiluz ?

This is needed for #7026

Thanks

/**
* @return array<string,array<string,ActionDto>>
*/
public function getActionList(): array
Copy link
Contributor

Choose a reason for hiding this comment

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

getAllActions() instead of getActionList()?

Suggested change
public function getActionList(): array
public function getAllActions(): array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, indeed not a bad idea.

I'm waiting for an opinion from javiereguiluz on this

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