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

[WIP] Create query context on datagrid builder #3247

Closed
wants to merge 1 commit into from

Conversation

soullivaneuh
Copy link
Member

Fixes #3246.

TODO

  • Make $context accessible from datagrid builder
  • Handle AdminInterface and DatagridBuilderInterface BC break
  • Adapt ORM datagrid builders to new API

Goal

The goal is to allow passing $context on datagrid builder to be used on Admin::createQuery.

Concrete case

I have a Ticket entity for dashboard support with feedback fields.

I want two list, the classic one and a special one with only feedbacked ones.

For this, I override Admin::createQuery to manage different contexts:

public function createQuery($context = 'list')
{
    /** @var QueryBuilder|ProxyQueryInterface $query */
    $query = parent::createQuery($context);

    if (in_array($context, ['list', 'feedback'], true)) {
        $query->orderBy('o.state', 'ASC');
        $query->addOrderBy('o.priority', 'DESC');
        $query->addOrderBy('o.id', 'DESC');
    }

    // Only ticket with feedback
    if ('feedback' === $context) {
        $query->andWhere('o.feedbackScore IS NOT NULL');
    }

    return $query;
}

And, in the feedback action controller, I call same logic as list but with the feedback context:

public function feedbackAction(Request $request)
{
    if (false === $this->admin->isGranted('LIST')) {
        throw $this->createAccessDeniedException();
    }

    $datagrid = $this->admin->getDatagrid('feedback');
    $formView = $datagrid->getForm()->createView();
    $this->get('twig')->getExtension('form')->renderer->setTheme($formView, $this->admin->getFilterTheme());

    return $this->render($this->admin->getTemplate('feedback'), [
        'action'     => 'feedback',
        'form'       => $formView,
        'datagrid'   => $datagrid,
    ], null, $request);
}

Without this PR, this is not possible and $context is completely useless AFAIK.

@rande What do you think? Am I right?

Also, a BC break was introduced on AdminInterface and DatagridBuilderInterface. I don't know how to handle it properly, any suggestion?

@soullivaneuh
Copy link
Member Author

ping @rande for opinion. :-)

@soullivaneuh
Copy link
Member Author

@rande Could you gave me a quick feedback to tell me if I'm going right? With that I could finish this PR. Thanks!

@soullivaneuh
Copy link
Member Author

ping @rande

@greg0ire
Copy link
Contributor

Hi @soullivaneuh ! As with discussed together, this encourages OCP violations. Here you only have two listings, but let's imagine you have n listings. Building a giant switch for the n listing inside createQuery is not a very good design. Instead, you could inject a custom listing object into the admin in your actions and call this object in createQuery. I think the $context parameter should just be removed.

@core23 core23 added the major label Apr 15, 2016
@soullivaneuh
Copy link
Member Author

Instead, you could inject a custom listing object into the admin in your actions and call this object in createQuery.

@greg0ire Do you have a concrete case with code to show?

@greg0ire
Copy link
Contributor

greg0ire commented May 8, 2016

@greg0ire Do you have a concrete case with code to show?

As a matter of fact, I do :)

I have this app that manages libraries (actual libraries, not code libraries),
and there's two listing views : one that is normal, and one that shows other
information for monitoring the libraries.

So I wrote two listing classes, that look like this :

<?php
namespace MyProject\Admin\Library;

use MyProject\Admin\FieldDescription\ConsumptionCountFieldDescription;
use MyProject\Admin\FieldDescription\ServiceStartupDateFieldDescription;
use MyProject\Admin\FieldDescription\SubscriberCountFieldDescription;
use MyProject\Admin\SourceIterator\MonitoredLibrarySourceIterator;
use MyProject\Entity\OfferRepository;
use Sonata\AdminBundle\Admin\AdminExtension;
use Sonata\AdminBundle\Datagrid\Datagrid;
use Sonata\AdminBundle\Datagrid\DatagridMapper;
use Sonata\AdminBundle\Datagrid\ListMapper;
use Symfony\Component\Translation\TranslatorInterface;

class MonitoringListing extends AdminExtension implements LibraryListingInterface
{
    private $offerRepository;

    public function __construct(OfferRepository $offerRepository)
    {
        $this->offerRepository = $offerRepository;
    }

    public function getRouteName()
    {
        return 'monitoring';
    }

    public function configureListFields(ListMapper $listMapper)
    {
        $listMapper
            ->add('libraryOffers')
            ->add('fake', null, [
                'template' => 'MyProject:List:list_inverted_boolean.html.twig'
            ])
            ->add('createdAt', null, ['format' => 'd/m/Y H:i:s']);
        $listMapper
            ->add(new SubscriberCountFieldDescription(true)) // with range filter
            ->add(new SubscriberCountFieldDescription(false)); //without
    }

    public function configureDatagridFilters(DatagridMapper $datagridMapper)
    {
        $datagridMapper
            ->add('native')
            ->add('active')
            ->add('fake')
            ->add(
                'column_createdAt',
                /* cache values, but do not apply them, they will be applied on
                another request (they should apply on aggregate columns) */
                'date_range_cache',
                [],
                'sonata_type_datepicker_range',
                [
                    'attr' => [
                        'label_start' => 'date_range.start',
                        'label_end'  => 'date_range.end'
                    ],
                    'dp_language' => 'fr',
                    'format'      => 'dd/MM/yyyy',
                    'dp_use_current' => false
                ]
            );
    }

    public function getExportRouteName()
    {
        return 'monitoring_export';
    }

    public function getDataSourceIterator(
        Datagrid $datagrid,
        TranslatorInterface $translator,
        array $libraries
    ) {
        $startupDescs = [];
        $consumptionDescs = [];
        foreach ($this->offerRepository->findAll() as $offer) {
            $startupDescs[$offer->getPortalApiUsername()] = new ServiceStartupDateFieldDescription($offer);
            $consumptionDescs[$offer->getPortalApiUsername()] = new ConsumptionCountFieldDescription($offer);
        }

        return new MonitoredLibrarySourceIterator(
            $datagrid,
            $translator,
            $libraries,
            $startupDescs,
            $consumptionDescs,
            new SubscriberCountFieldDescription(true),
            new SubscriberCountFieldDescription(false),
            $this->offerRepository
        );
    }
}

Here is an excerpt of the library controller:

<?php

namespace MyProject\Controller\Admin;

use MyProject\Admin\Library\MainListing;
use MyProject\Counter\ActiveLibraryCounter;
use MyProject\Counter\LegitEntityCounter;
use Sonata\AdminBundle\Controller\CRUDController;
use Sonata\AdminBundle\Exception\ModelManagerException;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;

class LibraryController extends CRUDController
{
    public function monitoringAction()
    {
        $this->admin->setListing($this->get('library.monitoring_listing'));

        return $this->showListing();
    }

    public function listAction()
    {
        $this->admin->setListing(new MainListing());

        return $this->showListing();
    }

    public function monitoringExportAction(Request $request)
    {
        $this->admin->setListing($this->get('library.monitoring_listing'));

        return parent::exportAction($request);
    }

    private function showListing()
    {
        if (false === $this->admin->isGranted('LIST')) {
            throw new AccessDeniedException();
        }

        $datagrid = $this->admin->getDatagrid();
        $formView = $datagrid->getForm()->createView();

        // set the theme for the current Admin Form
        $this->get('twig')->getExtension('form')->renderer->setTheme($formView, $this->admin->getFilterTheme());

        $legitLibraryCounter = new LegitEntityCounter($this->admin->getDatagrid()->getPager());
        $activeLibraryCounter = new ActiveLibraryCounter($this->admin->getDatagrid()->getPager());

        return $this->render($this->admin->getTemplate('list'), [
            'action' => 'list',
            'form' => $formView,
            'datagrid' => $datagrid,
            'csrf_token' => $this->getCsrfToken('sonata.batch'),
            'legitLibraryCount' => count($legitLibraryCounter),
            'activeLibraryCount' => count($activeLibraryCounter),
        ]);
    }

This helps keeping things relatively clean.

@soullivaneuh
Copy link
Member Author

Thanks @greg0ire, I'll try your solution on a concrete case and will give you a feedback. :-)

@soullivaneuh
Copy link
Member Author

@greg0ire On your code you have implements LibraryListingInterface but no reference to this interface.

From where this one come from?

@greg0ire
Copy link
Contributor

@greg0ire On your code you have implements LibraryListingInterface but no reference to this interface.

It's an interface of mine, you don't have to implement it. It's use for doing some type hinting, that's all…

@soullivaneuh
Copy link
Member Author

Closed. See #3795.

@soullivaneuh soullivaneuh deleted the datagrid-context branch May 11, 2016 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants