Add publication dates on pages #74

maximehuran · 2024-09-24T14:53:54Z

Publication dates

Form

Grid

No filter on published now

We have title with information

Enabled and not published

404

Enabled and published

200

Show in shop is disable if page is not published

Preview still works

maximehuran · 2024-09-24T15:21:07Z

src/Resources/config/services.yaml

@@ -35,3 +35,7 @@ services:
    MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext:
        decorates: router.request_context
        arguments: ['@MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext.inner']
+
+    Sylius\Calendar\Provider\DateTimeProviderInterface:


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

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

delyriand · 2024-09-25T06:52:21Z

src/Repository/PageRepositoryInterface.php

+    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;


💭 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?‏

delyriand · 2024-09-25T06:56:02Z

src/Resources/config/services.yaml

@@ -35,3 +35,7 @@ services:
    MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext:
        decorates: router.request_context
        arguments: ['@MonsieurBiz\SyliusCmsPagePlugin\Routing\RequestContext.inner']
+
+    Sylius\Calendar\Provider\DateTimeProviderInterface:


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

delyriand · 2024-09-25T06:58:29Z

src/Fixture/Factory/PageFixtureFactory.php

+            ->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;
+            })


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

delyriand · 2024-09-25T07:00:43Z

src/Fixture/Factory/PageFixtureFactory.php

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


🤓 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']);

```‏

etienne-monsieurbiz · 2024-09-25T15:58:58Z

src/Repository/PageRepository.php

@@ -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


💭 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

but it would be a BC

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.

etienne-monsieurbiz · 2024-09-25T15:59:25Z

src/Repository/PageRepository.php

@@ -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


💭 thought: same as my other comment‏

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?

etienne-monsieurbiz · 2024-09-25T16:05:05Z

src/Resources/views/Admin/Page/_showInShopButton.html.twig

@@ -1,7 +1,7 @@
 {% set enabledChannels = resource.channels|filter(channel => channel.enabled == true) %}

 {% if sylius_bundle_loaded_checker('SyliusShopBundle') %}
-    {% if not resource.enabled or enabledChannels|length < 1 %}
+    {% if not resource.enabled or not resource.isPublished(date()) or enabledChannels|length < 1 %}


💭 thought: Maybe we should keep the button even if the page is not published. This way you can check fast if your page is available on front or not.‏

etienne-monsieurbiz · 2024-09-25T16:07:37Z

src/Routing/PageSlugConditionChecker.php

        PageRepositoryInterface $pageRepository,
        ChannelContextInterface $channelContext,
-        LocaleContextInterface $localeContext
+        LocaleContextInterface $localeContext,
+        DateTimeProviderInterface $dateTimeProvider


💣 chore: Let's put this php 8 style. This version does not support php 7.4 anyway ‏

It could be a global refactoring could be better

maximehuran added 8 commits September 24, 2024 16:51

Add publication dates on pages entities

68616b8

Update page fixtures with publication dates

e9419d8

Update page form with publication dates

713487e

Manage pages grid in admin with publication date

686ca3c

Update page repository to deals with publication dates

d1ef1b9

Add translations for publication dates feature

f45856f

Update page repository to use date in function and not in construct

78302a6

Show in shop is disabled if page is not published

0cb86db

maximehuran requested review from delyriand, welcoMattic, lanfisis and etienne-monsieurbiz September 24, 2024 15:18

madamebiz added the Status: needs review Status: needs review label Sep 24, 2024

maximehuran commented Sep 24, 2024

View reviewed changes

delyriand reviewed Sep 25, 2024

View reviewed changes

etienne-monsieurbiz requested changes Sep 25, 2024

View reviewed changes

madamebiz added Status: needs update This Issue/PR needs update and removed Status: needs review Status: needs review labels Sep 25, 2024

lanfisis removed their request for review September 30, 2024 07:13

maximehuran mentioned this pull request Oct 30, 2024

New feature: draft #77

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add publication dates on pages #74

Add publication dates on pages #74

maximehuran commented Sep 24, 2024 •

edited

Loading

maximehuran Sep 24, 2024

delyriand Sep 25, 2024

delyriand Sep 25, 2024

delyriand Sep 25, 2024

delyriand Sep 25, 2024

delyriand Sep 25, 2024

etienne-monsieurbiz Sep 25, 2024

etienne-monsieurbiz Sep 25, 2024

welcoMattic Sep 26, 2024

etienne-monsieurbiz Sep 25, 2024

welcoMattic Sep 26, 2024

etienne-monsieurbiz Sep 25, 2024

etienne-monsieurbiz Sep 25, 2024

maximehuran Oct 9, 2024

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

Add publication dates on pages #74

Are you sure you want to change the base?

Add publication dates on pages #74

Conversation

maximehuran commented Sep 24, 2024 • edited Loading

Publication dates

Form

Grid

Enabled and not published

Enabled and published

Show in shop is disable if page is not published

Preview still works

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximehuran commented Sep 24, 2024 •

edited

Loading