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.

Sign up for GitHub

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

Prepare for BlockBundle 4.0 support and fix #1731 #1732

Closed
wants to merge 16 commits into from
Closed

Prepare for BlockBundle 4.0 support and fix #1731 #1732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a407784
fix #1731
haivala Apr 6, 2020
69de70b
remove one comment and fix lint
haivala Apr 6, 2020
db3704e
fix lint
haivala Apr 6, 2020
030f041
remove comments
haivala Apr 6, 2020
4be6b0f
add validateBlock too so block can be created
haivala Apr 6, 2020
7ed46d7
Add EditabBlockService to MediaBlockService
haivala Apr 8, 2020
9541daf
fix lint error
haivala Apr 8, 2020
7df47c0
Add Methods to all blocks
haivala Apr 8, 2020
c4a7145
fix lint
haivala Apr 8, 2020
116f48d
fix lint
haivala Apr 8, 2020
2652129
delete it -> Remove this method
haivala Apr 8, 2020
bee56aa
depr messages and $formMapper->getAdmin() to $this->container->get('s…
haivala Apr 8, 2020
2cdc1d3
CS-fixer
haivala Apr 8, 2020
8050085
cs
haivala Apr 8, 2020
f4eefb3
dont break 3.0
haivala Apr 9, 2020
3ddc959
remove comma
haivala Apr 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Block/FeatureMediaBlockService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Sonata\MediaBundle\Block;

use Sonata\AdminBundle\Form\FormMapper;
use Sonata\BlockBundle\Form\Mapper\FormMapper;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use Sonata\BlockBundle\Form\Mapper\FormMapper;
use Sonata\AdminBundle\Form\Mapper\FormMapper as AdminFormMapper;
use Sonata\BlockBundle\Form\Mapper\FormMapper;

Use AdminFormMapper in buildCreateForm and buildEditForm
Use FormMapper in configureCreateForm and configureEditForm

This can make some problem, I will check it.

use Sonata\BlockBundle\Meta\Metadata;
use Sonata\BlockBundle\Model\BlockInterface;
use Sonata\Form\Type\ImmutableArrayType;
Expand Down
87 changes: 80 additions & 7 deletions src/Block/GalleryBlockService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Sonata\AdminBundle\Admin\AdminInterface;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\AdminBundle\Form\Type\ModelListType;
use Sonata\BlockBundle\Block\BlockContextInterface;
use Sonata\BlockBundle\Block\Service\AbstractBlockService;
use Sonata\BlockBundle\Block\Service\EditableBlockService;
use Sonata\BlockBundle\Form\Mapper\FormMapper;
use Sonata\BlockBundle\Meta\Metadata;
use Sonata\BlockBundle\Meta\MetadataInterface;
use Sonata\BlockBundle\Model\BlockInterface;
use Sonata\Doctrine\Model\ManagerInterface;
use Sonata\Form\Type\ImmutableArrayType;
use Sonata\Form\Validator\ErrorElement;
use Sonata\MediaBundle\Model\GalleryInterface;
use Sonata\MediaBundle\Model\MediaInterface;
use Sonata\MediaBundle\Provider\Pool;
Expand All @@ -41,7 +44,7 @@
*
* @author Thomas Rabaix <[email protected]>
*/
class GalleryBlockService extends AbstractBlockService
class GalleryBlockService extends AbstractBlockService implements EditableBlockService
{
/**
* @var ManagerInterface
Expand Down Expand Up @@ -112,10 +115,12 @@ public function configureSettings(OptionsResolver $resolver)
]);
}

/**
* {@inheritdoc}
*/
public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
public function configureCreateForm(FormMapper $formMapper, BlockInterface $block)
{
$this->configureEditForm($formMapper, $block);
}

public function configureEditForm(FormMapper $formMapper, BlockInterface $block)
{
$contextChoices = [];

Expand All @@ -140,7 +145,7 @@ public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
'translation_domain' => 'SonataMediaBundle',
]);
$fieldDescription->setAssociationAdmin($this->getGalleryAdmin());
$fieldDescription->setAdmin($formMapper->getAdmin());
$fieldDescription->setAdmin($this->container->get('sonata.page.admin.block'));
Copy link
Member

Choose a reason for hiding this comment

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

It is OK for old Symfony practice.

For new pratice we should avoid using container. Please move it this service to block property and set it in constructor. This will allow inject required servaices insted container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the container should be removed in next major not in this patch. The container is used else where in the blocks too for example when we are getting the media admin. Can we have another pull request for that?

Copy link
Member

Choose a reason for hiding this comment

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

Container should be remove in Block 4 support becouse you must change constructor parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well this can be changed to be "prepare for Block 4 support".
I would happily do this later.
This was supposed to be patch for these blocks to work at all and now they work.
OK?

$fieldDescription->setOption('edit', 'list');
$fieldDescription->setAssociationMapping(['fieldName' => 'gallery', 'type' => ClassMetadataInfo::MANY_TO_ONE]);

Expand Down Expand Up @@ -196,6 +201,56 @@ public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
]);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function buildCreateForm(FormMapper $formMapper, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::configureCreateForm() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}
$this->configureCreateForm($formMapper, $block);
haivala marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::configureEditForm() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}
$this->configureEditForm($formMapper, $block);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function validateBlock(ErrorElement $errorElement, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::validate() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}

$this->validate($errorElement, $block);
}

public function validate(ErrorElement $errorElement, BlockInterface $block)
{
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -244,8 +299,26 @@ public function preUpdate(BlockInterface $block)
/**
* {@inheritdoc}
*/
public function getMetadata(): MetadataInterface
{
return new Metadata($this->getName(), null, null, 'SonataMediaBundle', [
'class' => 'fa fa-picture-o',
]);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function getBlockMetadata($code = null)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::getMetadata() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}

return new Metadata($this->getName(), (null !== $code ? $code : $this->getName()), false, 'SonataMediaBundle', [
'class' => 'fa fa-picture-o',
]);
Expand Down
84 changes: 78 additions & 6 deletions src/Block/GalleryListBlockService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@

namespace Sonata\MediaBundle\Block;

use Sonata\AdminBundle\Form\FormMapper;
use Sonata\BlockBundle\Block\BlockContextInterface;
use Sonata\BlockBundle\Block\Service\AbstractBlockService;
use Sonata\BlockBundle\Block\Service\EditableBlockService;
use Sonata\BlockBundle\Form\Mapper\FormMapper;
use Sonata\BlockBundle\Meta\Metadata;
use Sonata\BlockBundle\Meta\MetadataInterface;
use Sonata\BlockBundle\Model\BlockInterface;
use Sonata\Form\Type\ImmutableArrayType;
use Sonata\Form\Validator\ErrorElement;
use Sonata\MediaBundle\Model\GalleryManagerInterface;
use Sonata\MediaBundle\Provider\Pool;
use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;
Expand All @@ -32,7 +35,7 @@
/**
* @final since sonata-project/media-bundle 3.21.0
*/
class GalleryListBlockService extends AbstractBlockService
class GalleryListBlockService extends AbstractBlockService implements EditableBlockService
{
/**
* @var GalleryManagerInterface
Expand All @@ -57,10 +60,12 @@ public function __construct($twigOrName, ?EngineInterface $templating, GalleryMa
$this->pool = $pool;
}

/**
* {@inheritdoc}
*/
public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
public function configureCreateForm(FormMapper $formMapper, BlockInterface $block)
{
$this->configureEditForm($formMapper, $block);
}

public function configureEditForm(FormMapper $formMapper, BlockInterface $block)
{
$contextChoices = [];

Expand Down Expand Up @@ -122,6 +127,55 @@ public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
]);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function buildCreateForm(FormMapper $formMapper, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::configureCreateForm() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}
$this->configureCreateForm($formMapper, $block);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function buildEditForm(FormMapper $formMapper, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::configureEditForm() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}
$this->configureEditForm($formMapper, $block);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function validateBlock(ErrorElement $errorElement, BlockInterface $block)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::validate() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}
$this->validate($errorElement, $block);
}

public function validate(ErrorElement $errorElement, BlockInterface $block)
{
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -173,8 +227,26 @@ public function configureSettings(OptionsResolver $resolver)
/**
* {@inheritdoc}
*/
public function getMetadata(): MetadataInterface
{
return new Metadata($this->getName(), null, null, 'SonataMediaBundle', [
'class' => 'fa fa-picture-o',
]);
}

/**
* NEXT_MAJOR: Remove this method.
*/
public function getBlockMetadata($code = null)
{
if ('sonata_deprecation_mute' !== (\func_get_args()[2] ?? null)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this 2 need to be changed to 1?

Copy link
Member

Choose a reason for hiding this comment

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

It depends on how are you providing the arguments for this method, but since there is no magic usage for argument 2 (offset 1 for func_get_args()), I think you must use it.

@trigger_error(sprintf(
'Method %s() is deprecated since sonata-project/media-bundle 3.x and will be removed in version 4.0. Use %s::getMetadata() instead.',
__METHOD__,
static::class
), E_USER_DEPRECATED);
}

return new Metadata($this->getName(), (null !== $code ? $code : $this->getName()), false, 'SonataMediaBundle', [
'class' => 'fa fa-picture-o',
]);
Expand Down
Loading