DrupalDateTime serialization issue

Thanks, patch looks straightforward, just need a test

Looking good, would be good to somehow see a failing test patch

I was recently dealing with serializing issues in migration tests, so I have thrown in my 2 cents.
Also, I'm +1 to upload a test-only patch and moving to NW to address remaining items.

For some reason, GitLab didn't send me a notification about your comment @mohit_rocks...
I have just one more question in gitlab ;)

I'm also wondering why the target branch is 8.9.x. Shouldn't it be 9.2.x?

Re-rolled patch for 9.2.x

Couldn't figure out what was going on in the merge request. 1000s of changes and commits so maybe it needs to rebased or something.

If I understand the problem correctly, this is just on the property caching localisation. Looking at the patch, could we avoid a lot of complexity with the trait and wakeup stuff and just empty the cache during serialization and assume it can(and should) be rebuilt after serialization? The rest shouldn't really matter.

a.dmitriiev → closed merge request !112

I created another MR to target to 9.3.x branch.

In my particular case the DrupalDateTime object was used in ajax form and the form cache was broken because of date object serialization, because localization has database service.

Drupal 9.3.0-rc1 → was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Thanks for the MR and patch. If I understand correctly, the patch is for 8.9 and the MR is for 9.4.

I looked at the patch in #11 and noticed a couple minor nitpicks:

1. +++ b/core/lib/Drupal/Core/Datetime/DrupalDateTime.php
   @@ -21,6 +22,10 @@
   +    __wakeup as defaultWakeup;
   +    __sleep as defaultSleep;
   

   Nitpick: Ordering doesn't match function ordering (__sleep then __wakeup) but I see this is the same as core/modules/forum/src/ForumManager.php so I guess it should stay.

2. +++ b/core/tests/Drupal/Tests/Core/Datetime/DrupalDateTimeTest.php
   @@ -239,4 +239,23 @@ public function testGetPhpDateTime() {
   +   * Test to avoid serialization of formatTranslationCache
   

   Period is missing.

The MR has a gazillion changes so I'm not sure what to do there: https://git.drupalcode.org/project/drupal/-/merge_requests/112.diff

Just fixed CS error

Kristen, the MR 122 is already closed because it is outdated. The new MR is here https://git.drupalcode.org/project/drupal/-/merge_requests/1340

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened → , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Drupal 9.5.0-beta2 → and Drupal 10.0.0-beta2 → were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Drupal 9.4.0-alpha1 → was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .

Moving back to needs work based on feedback above.

recrit → changed the visibility of the branch 11.x to hidden.

recrit → changed the visibility of the branch 3187004-drupal9-rebased to hidden.

I created a new MR 6334 based on 11.x. The following have been address.

- #18 code cleanup 🐛 DrupalDateTime serialization issue Fixed - DONE..

- Regarding MR1340 comment "Can we replace this by just settings the property definition to protected $formatTranslationCache = [];" - DONE

- Regarding MR1340 comment "This class really doesn't need any of the service detection magic. Lets just do this and get rid of the trait." - This does need the DependencySerializationTrait for the service "string_translation" stored in protected $stringTranslation.

attaching a static patch for composer builds.

Left some nitpicky stuff on the MR.

@smustgrave All MR comments have been resolved

All feedback appears to be addressed!

Issue credits

larowlan → closed merge request !6334

Committed to 11.x and backported to 10.2.x - thanks!

Automatically closed - issue fixed for 2 weeks with no activity.