-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add the refresh_action #180
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Kreyu\Bundle\DataTableBundle\Action; | ||
|
||
use Kreyu\Bundle\DataTableBundle\Exception\LogicException; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\RequestStack; | ||
use Symfony\Component\Routing\Generator\UrlGeneratorInterface; | ||
|
||
class ActionRefreshUrlGenerator implements ActionRefreshUrlGeneratorInterface | ||
{ | ||
public function __construct( | ||
private RequestStack $requestStack, | ||
private UrlGeneratorInterface $urlGenerator, | ||
) { | ||
} | ||
|
||
public function generate(): string | ||
{ | ||
$request = $this->getRequest(); | ||
|
||
$route = $request->attributes->get('_route'); | ||
$routeParams = $request->attributes->get('_route_params', []); | ||
$queryParams = $request->query->all(); | ||
|
||
$parameters = [...$routeParams, ...$queryParams]; | ||
|
||
return $this->urlGenerator->generate($route, $parameters); | ||
} | ||
|
||
private function getRequest(): Request | ||
{ | ||
if (null === $request = $this->requestStack->getCurrentRequest()) { | ||
throw new LogicException('Unable to retrieve current request.'); | ||
} | ||
|
||
return $request; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
declare(strict_types=1); | ||
|
||
namespace Kreyu\Bundle\DataTableBundle\Action; | ||
|
||
interface ActionRefreshUrlGeneratorInterface | ||
{ | ||
public function generate(): string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Kreyu\Bundle\DataTableBundle\Action\Type; | ||
|
||
use Kreyu\Bundle\DataTableBundle\Action\ActionContext; | ||
use Kreyu\Bundle\DataTableBundle\Action\ActionInterface; | ||
use Kreyu\Bundle\DataTableBundle\Action\ActionRefreshUrlGeneratorInterface; | ||
use Kreyu\Bundle\DataTableBundle\Action\ActionView; | ||
|
||
class RefreshActionType extends AbstractActionType | ||
{ | ||
public function __construct( | ||
private ActionRefreshUrlGeneratorInterface $columnRefreshUrlGenerator, | ||
) { | ||
} | ||
|
||
public function buildView(ActionView $view, ActionInterface $action, array $options): void | ||
{ | ||
if (ActionContext::Global !== $action->getConfig()->getContext()) { | ||
throw new \LogicException(sprintf('A %s action can only be added as a global action.', $this::class)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call 👍🏻 It could be used as row or batch action, but that would be unintuitive. |
||
} | ||
|
||
$view->vars = array_replace($view->vars, [ | ||
'href' => $this->columnRefreshUrlGenerator->generate(), | ||
]); | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this function is needed, since that identifier is used primarily for the turbo frame. If I call the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand, but I don't like the idea of having an identification key that is hard-coded in a view and used in several different places. Do you have any suggestions on how to change this approach? Perhaps a more explicit function name, such as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this class (and its interface) at all? I think refreshing the data table should always use the current URL, which (I believe) we can retrieve directly in Twig?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our project, we sometimes return the same DataTable in multiple views. To avoid instantiating this DataTable in several places (so as to keep the same options), we created an endpoint that returns only the content of this DataTable.
I wanted to replicate this behavior.