Publish status and unpublish status field weights reset when schedule settings are editted

I need to spend a bit of time figuring out how to actually create a merge request in an issue. Adding a patch for now.

Ran into this same issue. Steps to reproduce were accurate and patch resolves issue.

Patch no longer applies to 2.x so wonder if this is still an issue there?

It's still needed. Just needs a reroll. #3359796: Skip form displays that don't exist. → added some lines above the function the patch was updating. I'll see if I can get a new patch posted today or tomorrow.

And here we go. New patch with new line numbers.

I have sorted out the MR branch, cherry-picked the missing commits that have been made to 2.x since this issue was opened, then added the changes in the new patch from joshuami in #7

Do we have any test coverage for these forms?

I think that the test failures in the GitLab pipeline (which run D10) are unrelated to this chnage. I have raised #3390920: Replace assert[Not]Equals() in tests → to deal with that problem.

I have fixed #3390920: Remove markup in assertEquals() and fix strToLower NULL deprecations → so this MR pipeline now passes green. We do not have any test coverage for this though.

Will admit a lot of the modules I have don't have test case (I know terrible practice!) but since this project does have tests think we should add one for this issue.

In a project using this patch, we just discovered an edge case that wasn't covered when scheduling media. It only occurs when creating new media, but the publish status sort weight changes back to 53 when the error is triggered before the media entity is saved. I can't reproduce this for content entities, so it may be an issue upstream in core media. I'll try an dig into this a little more as time permits.

Fixes will need to be against 3.0.x

Created a MR against 3.0.x and hiding the previous branches.

This patch still works on on 3.0.3. I didn't make any updates.

#14 still has an open question.
Still needs test coverage, per #13.

capysara → changed the visibility of the branch 3365161-publish-status-and to hidden.

capysara → changed the visibility of the branch 3.0.x to hidden.

capysara → changed the visibility of the branch 3365161-publish-status-and-patch-3ac7 to hidden.

Per #13 → I added a functional test to confirm the field weight reset fix.

Branch seems to have failing tests but don't appear to be related.

Just re-ran the 3.0.x pipeline and minus some phpcs for use statements the pipeline tests are green. So these failures may be legit.

@smustgrave I went ahead and tried running all the module tests locally with the latest 3.x dev branch. The LayoutBuilderTest functional test always failed for me, and all the other tests passed, with and without this issue's MR changes added. The new FormDisplayWeightTest test always passed for me with the MR changed applied.

Below is the error from the failing LayoutBuilderTest test. I get the same error when I ran just the LayoutBuilderTest test by itself.

Testing /var/www/html/docroot/modules/contrib/scheduler_content_moderation_integration
....................E...............................              52 / 52 (100%)

Time: 06:39.901, Memory: 8.00 MB

There was 1 error:

1) Drupal\Tests\scheduler_content_moderation_integration\Functional\LayoutBuilderTest::testLayoutBuilder
Behat\Mink\Exception\ExpectationException: Current response status code is 500, but 200 expected.

/var/www/html/vendor/behat/mink/src/WebAssert.php:888
/var/www/html/vendor/behat/mink/src/WebAssert.php:145
/var/www/html/docroot/modules/contrib/scheduler_content_moderation_integration/tests/src/Functional/LayoutBuilderTest.php:44
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

ERRORS!
Tests: 52, Assertions: 1356, Errors: 1.

I did also got the following deprecation notice when running the tests:

Remaining self deprecation notices (1)

  1x: system_time_zones() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. This function is no longer used in Drupal core. Use \Drupal\Core\Datetime\TimeZoneFormHelper::getOptionsList(), \Drupal\Core\Datetime\TimeZoneFormHelper::getOptionsListByRegion() or \DateTimeZone::listIdentifiers() instead. See https://www.drupal.org/node/3023528
    1x in LayoutBuilderTest::testLayoutBuilder from Drupal\Tests\scheduler_content_moderation_integration\Functional

Would a test fail because of the deprecation notice?

I ran all the modules tests about a half-dozen times with the same results.

That’s odd? I merged in an issue yesterday which ran the pipeline probably 3-5 times no issue

The failing test may be caused by the Twig version, the composer job log shows twig/twig (v3.14.1) which has a timeout problem, and woud cause the problem of not finding expected elements on the form. The previous pipeline on this MR had (v3.14.0) which does not have the problem. However, jobs today should be using v3.14.2 which was released yesterday to fix the problem. Is this project locked to twig 3.14.1 ? 💬 Updating phpstan and twig via Composer generates a blank page when editing a node Active has the background.

The failing test may be caused by the Twig version, the composer job log shows twig/twig (v3.14.1) which has a timeout problem, and woud cause the problem of not finding expected elements on the form. The previous pipeline on this MR had (v3.14.0) which does not have the problem.

Yeah, looks like that was the problem. I re-ran the composer build stage and that installed twig/twig (v3.14.2). Tests seem to be passing now.