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

Add publication dates on pages #74

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
30 changes: 30 additions & 0 deletions src/Entity/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ class Page implements PageInterface
*/
protected $updatedAt;

protected ?DateTimeInterface $publishAt = null;

protected ?DateTimeInterface $unpublishAt = null;

/**
* Page constructor.
*/
Expand Down Expand Up @@ -115,6 +119,32 @@ public function hasChannel(ChannelInterface $channel): bool
return $this->channels->contains($channel);
}

public function getPublishAt(): ?DateTimeInterface
{
return $this->publishAt;
}

public function setPublishAt(?DateTimeInterface $publishAt): void
{
$this->publishAt = $publishAt;
}

public function getUnpublishAt(): ?DateTimeInterface
{
return $this->unpublishAt;
}

public function setUnpublishAt(?DateTimeInterface $unpublishAt): void
{
$this->unpublishAt = $unpublishAt;
}

public function isPublished(DateTimeInterface $dateTime): bool
{
return (null == $this->publishAt || $this->publishAt <= $dateTime)
&& (null === $this->unpublishAt || $this->unpublishAt > $dateTime);
}

public function getTitle(): ?string
{
return $this->getTranslation()->getTitle();
Expand Down
11 changes: 11 additions & 0 deletions src/Entity/PageInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace MonsieurBiz\SyliusCmsPagePlugin\Entity;

use DateTimeInterface;
use Gedmo\Timestampable\Timestampable;
use Sylius\Component\Channel\Model\ChannelsAwareInterface;
use Sylius\Component\Resource\Model\CodeAwareInterface;
Expand All @@ -33,6 +34,16 @@ public function getCode(): ?string;

public function setCode(?string $title): void;

public function getPublishAt(): ?DateTimeInterface;

public function setPublishAt(?DateTimeInterface $publishAt): void;

public function getUnpublishAt(): ?DateTimeInterface;

public function setUnpublishAt(?DateTimeInterface $unpublishAt): void;

public function isPublished(DateTimeInterface $dateTime): bool;

public function getTitle(): ?string;

public function setTitle(?string $title): void;
Expand Down
19 changes: 19 additions & 0 deletions src/Fixture/Factory/PageFixtureFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace MonsieurBiz\SyliusCmsPagePlugin\Fixture\Factory;

use DateTime;
use MonsieurBiz\SyliusCmsPagePlugin\Entity\PageInterface;
use MonsieurBiz\SyliusCmsPagePlugin\Entity\PageTranslationInterface;
use Sylius\Bundle\CoreBundle\Fixture\Factory\AbstractExampleFactory;
Expand Down Expand Up @@ -85,6 +86,16 @@ public function create(array $options = []): PageInterface
$page->setEnabled($options['enabled']);
$page->setCode($options['code']);

$publishAt = $options['publish_at'] ?? null;
if ($publishAt) {
$page->setPublishAt(new DateTime($publishAt));
}

$unpublishAt = $options['unpublish_at'] ?? null;
if ($unpublishAt) {
$page->setUnpublishAt(new DateTime($unpublishAt));
}
Comment on lines +89 to +97
Copy link
Member

Choose a reason for hiding this comment

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

🤓 nitpick: if you add the normalizer, unnecessary conditions

Suggested change
$publishAt = $options['publish_at'] ?? null;
if ($publishAt) {
$page->setPublishAt(new DateTime($publishAt));
}
$unpublishAt = $options['unpublish_at'] ?? null;
if ($unpublishAt) {
$page->setUnpublishAt(new DateTime($unpublishAt));
}
$page->setPublishAt($options['publish_at']);
$page->setUnpublishAt($options['unpublish_at']);
```‏


foreach ($options['channels'] as $channel) {
$page->addChannel($channel);
}
Expand Down Expand Up @@ -116,6 +127,8 @@ private function createTranslations(PageInterface $page, array $options): void
*/
protected function configureOptions(OptionsResolver $resolver): void
{
$publishAt = $this->faker->dateTimeBetween('-1 year', '+1 year');
$hasPublishAt = $this->faker->boolean(20);
$resolver
->setDefault('enabled', function (Options $options): bool {
return $this->faker->boolean(80);
Expand All @@ -126,6 +139,12 @@ protected function configureOptions(OptionsResolver $resolver): void
->setDefault('translations', function (OptionsResolver $translationResolver): void {
$translationResolver->setDefaults($this->configureDefaultTranslations());
})
->setDefault('publish_at', function (Options $options) use ($publishAt, $hasPublishAt): ?string {
return $hasPublishAt ? $publishAt->format('Y-m-d H:i:s') : null;
})
->setDefault('unpublish_at', function (Options $options) use ($publishAt): ?string {
return $this->faker->boolean(20) ? (clone $publishAt)->modify('+' . $this->faker->numberBetween(1, 20) . ' days')->format('Y-m-d H:i:s') : null;
})
Comment on lines +142 to +147
Copy link
Member

Choose a reason for hiding this comment

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

🤓 nitpick: use a setNormalizer to transform to DateTime if is a string

->setDefault('channels', LazyOption::all($this->channelRepository))
->setAllowedTypes('channels', 'array')
->setNormalizer('channels', LazyOption::findBy($this->channelRepository, 'code'))
Expand Down
2 changes: 2 additions & 0 deletions src/Fixture/PageFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ protected function configureResourceNode(ArrayNodeDefinition $resourceNode): voi
->children()
->booleanNode('enabled')->end()
->scalarNode('code')->cannotBeEmpty()->end()
->scalarNode('publish_at')->end()
->scalarNode('unpublish_at')->end()
->arrayNode('channels')
->scalarPrototype()->end()
->end()
Expand Down
11 changes: 11 additions & 0 deletions src/Form/Type/PageType.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Sylius\Bundle\ResourceBundle\Form\Type\AbstractResourceType;
use Sylius\Bundle\ResourceBundle\Form\Type\ResourceTranslationsType;
use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
use Symfony\Component\Form\Extension\Core\Type\DateTimeType;
use Symfony\Component\Form\FormBuilderInterface;

class PageType extends AbstractResourceType
Expand All @@ -33,6 +34,16 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'required' => false,
'label' => 'monsieurbiz_cms_page.ui.form.enabled',
])
->add('publishAt', DateTimeType::class, [
'label' => 'monsieurbiz_cms_page.ui.form.publish_at',
'required' => false,
'widget' => 'single_text',
])
->add('unpublishAt', DateTimeType::class, [
'label' => 'monsieurbiz_cms_page.ui.form.unpublish_at',
'required' => false,
'widget' => 'single_text',
])
->add('channels', ChannelChoiceType::class, [
'label' => 'monsieurbiz_cms_page.ui.form.channels',
'required' => false,
Expand Down
40 changes: 40 additions & 0 deletions src/Migrations/Version20240924135829.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/*
* This file is part of Monsieur Biz' Cms Page plugin for Sylius.
*
* (c) Monsieur Biz <sylius@monsieurbiz.com>
*
* For the full copyright and license information, please view the LICENSE.txt
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace MonsieurBiz\SyliusCmsPagePlugin\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20240924135829 extends AbstractMigration
{
public function getDescription(): string
{
return '';
}

public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE monsieurbiz_cms_page ADD publish_at DATETIME DEFAULT NULL, ADD unpublish_at DATETIME DEFAULT NULL');
}

public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE monsieurbiz_cms_page DROP publish_at, DROP unpublish_at');
}
}
11 changes: 9 additions & 2 deletions src/Repository/PageRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace MonsieurBiz\SyliusCmsPagePlugin\Repository;

use DateTimeInterface;
use Doctrine\ORM\NonUniqueResultException;
use Doctrine\ORM\QueryBuilder;
use MonsieurBiz\SyliusCmsPagePlugin\Entity\PageInterface;
Expand Down Expand Up @@ -48,11 +49,14 @@ public function existsOneByChannelAndSlug(ChannelInterface $channel, ?string $lo
return $count > 0;
}

public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug): bool
public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug, DateTimeInterface $dateTime): bool
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 thought: I think this method should be rename to represent more what it is doing‏

Suggested change
public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug, DateTimeInterface $dateTime): bool
public function existsOnePublishedByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug, DateTimeInterface $dateTime): bool

Copy link
Contributor

Choose a reason for hiding this comment

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

but it would be a BC

Copy link
Contributor

Choose a reason for hiding this comment

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

We can mark existsOneEnabledByChannelAndSlug as deprecated, create a new method named existsOnePublishedByChannelAndSlug that call existsOneEnabledByChannelAndSlug, then in the 2.0 version, let's remove existsOneEnabledByChannelAndSlug in favor of existsOnePublishedByChannelAndSlug.

{
$queryBuilder = $this->createQueryBuilderExistOne($channel, $locale, $slug);
$queryBuilder
->andWhere('p.enabled = true')
->andWhere('p.publishAt IS NULL OR p.publishAt <= :now')
->andWhere('p.unpublishAt IS NULL OR p.unpublishAt >= :now')
->setParameter('now', $dateTime)
;

$count = (int) $queryBuilder
Expand All @@ -66,7 +70,7 @@ public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?str
/**
* @throws NonUniqueResultException
*/
public function findOneEnabledBySlugAndChannelCode(string $slug, string $localeCode, string $channelCode): ?PageInterface
public function findOneEnabledBySlugAndChannelCode(string $slug, string $localeCode, string $channelCode, DateTimeInterface $dateTime): ?PageInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 thought: same as my other comment‏

Copy link
Contributor

Choose a reason for hiding this comment

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

To handle the BC here, we can make the argument nullable. If it's actually null, trigger a deprecation message to inform users this argument will become mandatory in 2.0. WDYT?

{
return $this->createQueryBuilder('p')
->leftJoin('p.translations', 'translation')
Expand All @@ -75,6 +79,9 @@ public function findOneEnabledBySlugAndChannelCode(string $slug, string $localeC
->andWhere('translation.slug = :slug')
->andWhere('channels.code = :channelCode')
->andWhere('p.enabled = true')
->andWhere('p.publishAt IS NULL OR p.publishAt <= :now')
->andWhere('p.unpublishAt IS NULL OR p.unpublishAt >= :now')
->setParameter('now', $dateTime)
->setParameter('localeCode', $localeCode)
->setParameter('slug', $slug)
->setParameter('channelCode', $channelCode)
Expand Down
5 changes: 3 additions & 2 deletions src/Repository/PageRepositoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace MonsieurBiz\SyliusCmsPagePlugin\Repository;

use DateTimeInterface;
use Doctrine\ORM\QueryBuilder;
use MonsieurBiz\SyliusCmsPagePlugin\Entity\PageInterface;
use Sylius\Component\Channel\Model\ChannelInterface;
Expand All @@ -24,7 +25,7 @@ public function createListQueryBuilder(string $localeCode): QueryBuilder;

public function existsOneByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug, array $excludedPages = []): bool;

public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug): bool;
public function existsOneEnabledByChannelAndSlug(ChannelInterface $channel, ?string $locale, string $slug, DateTimeInterface $dateTime): bool;

public function findOneEnabledBySlugAndChannelCode(string $slug, string $localeCode, string $channelCode): ?PageInterface;
public function findOneEnabledBySlugAndChannelCode(string $slug, string $localeCode, string $channelCode, DateTimeInterface $dateTime): ?PageInterface;
Comment on lines +28 to +30
Copy link
Member

Choose a reason for hiding this comment

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

💭 thought: it's a BC with this new parameter. Shouldn't we set it to null by default and throw a depreciation is the value is null?‏

}
2 changes: 2 additions & 0 deletions src/Resources/config/doctrine/Page.orm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
<option name="default">1</option>
</options>
</field>
<field name="publishAt" column="publish_at" type="datetime" nullable="true"/>
<field name="unpublishAt" column="unpublish_at" type="datetime" nullable="true"/>
<field name="createdAt" column="created_at" type="datetime_immutable">
<gedmo:timestampable on="create"/>
</field>
Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/routing/shop.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ monsieurbiz_cms_page_show:
- $slug
- "expr:service('sylius.context.locale').getLocaleCode()"
- "expr:service('sylius.context.channel').getChannel().getCode()"
- "expr:service('Sylius\\\\Calendar\\\\Provider\\\\DateTimeProviderInterface').now()"
requirements:
slug: .+
condition: "not(context.getPathInfo() matches '`^%sylius.security.new_api_route%`') and context.checkPageSlug(request)"
4 changes: 4 additions & 0 deletions src/Resources/config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ services:
MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext:
decorates: router.request_context
arguments: ['@MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext.inner']

Sylius\Calendar\Provider\DateTimeProviderInterface:
Copy link
Member Author

Choose a reason for hiding this comment

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

❔ question: ‏WDYTOT ? Else we can have a DateTimeProviderInterface in our namespace

Copy link
Member

Choose a reason for hiding this comment

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

We could create a custom alias or provider that takes the native one as an argument

class: Sylius\Calendar\Provider\Calendar
public: true
Loading
Loading