-
Notifications
You must be signed in to change notification settings - Fork 25
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
base: master
Are you sure you want to change the base?
Changes from all commits
68616b8
e9419d8
713487e
686ca3c
d1ef1b9
f45856f
78302a6
0cb86db
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 |
---|---|---|
|
@@ -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; | ||
|
@@ -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)); | ||
} | ||
|
||
foreach ($options['channels'] as $channel) { | ||
$page->addChannel($channel); | ||
} | ||
|
@@ -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); | ||
|
@@ -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
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. 🤓 nitpick: use a |
||
->setDefault('channels', LazyOption::all($this->channelRepository)) | ||
->setAllowedTypes('channels', 'array') | ||
->setNormalizer('channels', LazyOption::findBy($this->channelRepository, 'code')) | ||
|
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 <[email protected]> | ||
* | ||
* 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'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,6 +13,7 @@ | |||||
|
||||||
namespace MonsieurBiz\SyliusCmsPagePlugin\Repository; | ||||||
|
||||||
use DateTimeInterface; | ||||||
use Doctrine\ORM\NonUniqueResultException; | ||||||
use Doctrine\ORM\QueryBuilder; | ||||||
use MonsieurBiz\SyliusCmsPagePlugin\Entity\PageInterface; | ||||||
|
@@ -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 | ||||||
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. 💭 thought: I think this method should be rename to represent more what it is doing
Suggested change
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. but it would be a BC 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. We can mark |
||||||
{ | ||||||
$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 | ||||||
|
@@ -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 | ||||||
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. 💭 thought: same as my other comment 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. To handle the BC here, we can make the argument nullable. If it's actually |
||||||
{ | ||||||
return $this->createQueryBuilder('p') | ||||||
->leftJoin('p.translations', 'translation') | ||||||
|
@@ -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) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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
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. 💭 thought: it's a BC with this new parameter. Shouldn't we set it to |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,3 +35,7 @@ services: | |
MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext: | ||
decorates: router.request_context | ||
arguments: ['@MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext.inner'] | ||
|
||
Sylius\Calendar\Provider\DateTimeProviderInterface: | ||
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. ❔ question: WDYTOT ? Else we can have a 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. We could create a custom alias or provider that takes the native one as an argument |
||
class: Sylius\Calendar\Provider\Calendar | ||
public: true |
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.
🤓 nitpick: if you add the normalizer, unnecessary conditions