Problem/Motivation

drupal-check results on commit hash:
source : [git] https://git.drupal.org/project/workbench_moderation 661c2108c84e961a00ae7174c91053858447a7d2
source : http://cgit.drupalcode.org/workbench_moderation


 
Fatal error: Declaration of Drupal\workbench_moderation\Plugin\Menu\EditTab::getTitle() must be compatible with Drupal\Core\Menu\LocalTaskDefault::getTitle(?Symfony\Component\HttpFoundation\Request $request = NULL) in /Users/dwaynemcdaniel/Documents/d9-contrib/midcamp2019-drupal/web/modules/contrib/workbench_moderation/src/Plugin/Menu/EditTab.php on line 16 
 

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#30 3042757-30.patch65.56 KBlarowlan
#30 3042757-30-interdiff.txt34.92 KBlarowlan
#29 3042757-29.patch30.94 KBlarowlan
#29 3042757-29-interdiff.txt2.57 KBlarowlan
#28 3042757-28.patch28.37 KBlarowlan
#28 3042757-28-interdiff.txt2 KBlarowlan
#27 3042757-27.patch26.92 KBlarowlan
#27 3042757-27-interdiff.txt2.09 KBlarowlan
#26 3042757-27.patch24.82 KBlarowlan
#26 3042757-interdiff-27.txt3.65 KBlarowlan
#25 3042757-25.patch22.5 KBlarowlan
#23 3042757-23.patch22.5 KBlarowlan
#23 3042757-interdiff-23.txt3.25 KBlarowlan
#21 3042757-21.patch19.25 KBlarowlan
#21 interdiff-3042757-21.txt1.02 KBlarowlan
#16 3042757-interdiff-16.txt1.47 KBlarowlan
#16 3042757-16.patch18.22 KBlarowlan
#13 workbench_moderation-3042757-13.patch17.07 KBTolyan4ik
#4 workbench_moderation-3042757-4.patch1000 bytesjoy29
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mcdwayne created an issue. See original summary.

larowlan’s picture

Issue tags: +Novice
joy29’s picture

Assigned: Unassigned » joy29
joy29’s picture

joy29’s picture

Status: Active » Needs review
tatarbj’s picture

Issue tags: +DrupalCampBelarus2019
stevector’s picture

I ran the checker locally and got many more results than were reported here: https://github.com/mcdwayne/Drupal-9-drupal-check-Report/blob/master/Dru...


 -drupal-check web/modules/contrib/workbench_moderation
 69/69 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ---------------------------------------------------
  Line   src/Form/BundleModerationConfigurationForm.php
 ------ ---------------------------------------------------
  189    Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------
  Line   src/Form/EntityModerationForm.php
 ------ ---------------------------------------------------
  149    Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------
  Line   src/Form/ModerationStateDeleteForm.php
 ------ ---------------------------------------------------
  40     Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------
  Line   src/Form/ModerationStateForm.php
 ------ ---------------------------------------------------
  72     Call to deprecated function drupal_set_message().
  78     Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------
  Line   src/Form/ModerationStateTransitionDeleteForm.php
 ------ ---------------------------------------------------
  40     Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------
  Line   src/Form/ModerationStateTransitionForm.php
 ------ ---------------------------------------------------
  128    Call to deprecated function drupal_set_message().
  134    Call to deprecated function drupal_set_message().
 ------ ---------------------------------------------------

 ------ --------------------------------------------------------------------------
  Line   src/Permissions.php
 ------ --------------------------------------------------------------------------
  16     Usage of deprecated trait Drupal\Core\Routing\UrlGeneratorTrait in class
         Drupal\workbench_moderation\Permissions.
 ------ --------------------------------------------------------------------------

 ------ ----------------------------------------------------------------------------------------------
  Line   src/Plugin/Action/ModerationOptOutPublishNode.php
 ------ ----------------------------------------------------------------------------------------------
  18     Class Drupal\workbench_moderation\Plugin\Action\ModerationOptOutPublishNode extends
         deprecated class Drupal\node\Plugin\Action\PublishNode.
  26     Call to method __construct() of deprecated class Drupal\node\Plugin\Action\PublishNode.
  46     Call to deprecated function drupal_set_message().
  50     Call to method execute() of deprecated class Drupal\Core\Action\Plugin\Action\PublishAction.
  57     Call to method access() of deprecated class Drupal\Core\Action\Plugin\Action\PublishAction.
 ------ ----------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------
  Line   src/Plugin/Action/ModerationOptOutUnpublishNode.php
 ------ -----------------------------------------------------------------------------------------------
  18     Class Drupal\workbench_moderation\Plugin\Action\ModerationOptOutUnpublishNode extends
         deprecated class Drupal\node\Plugin\Action\UnpublishNode.
  26     Call to method __construct() of deprecated class Drupal\node\Plugin\Action\UnpublishNode.
  46     Call to deprecated function drupal_set_message().
  50     Call to method execute() of deprecated class
         Drupal\Core\Action\Plugin\Action\UnpublishAction.
  57     Call to method access() of deprecated class Drupal\Core\Action\Plugin\Action\UnpublishAction.
 ------ -----------------------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------------------
  Line   src/Routing/EntityModerationRouteProvider.php
 ------ -------------------------------------------------------------------------------------------
  110    Call to deprecated method isSubclassOf() of class Drupal\Core\Entity\EntityTypeInterface.
 ------ -------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------
  Line   tests/src/Functional/LatestRevisionViewsFilterTest.php
 ------ ------------------------------------------------------------------------------------------
  17     Class Drupal\Tests\workbench_moderation\Functional\LatestRevisionViewsFilterTest extends
         deprecated class Drupal\simpletest\BrowserTestBase.
 ------ ------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------
  Line   tests/src/Kernel/ViewsDataIntegrationTest.php
 ------ --------------------------------------------------------------------------------------------
         Class Drupal\Tests\workbench_moderation\Kernel\ViewsDataTest was not found while trying to
         analyse it - autoloading is probably not configured properly.
 ------ --------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------
  Line   tests/src/Kernel/WorkbenchModerationSchemaTest.php
 ------ -----------------------------------------------------------------------------
  19     Usage of deprecated trait Drupal\config\Tests\SchemaCheckTestTrait in class
         Drupal\Tests\workbench_moderation\Kernel\WorkbenchModerationSchemaTest.
 ------ -----------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   tests/src/Unit/LatestRevisionCheckTest.php
 ------ -------------------------------------------------------------------------
         Class PHPUnit_Framework_TestCase not found and could not be autoloaded.
 ------ -------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   tests/src/Unit/ModerationInformationTest.php
 ------ -------------------------------------------------------------------------
         Class PHPUnit_Framework_TestCase not found and could not be autoloaded.
 ------ -------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   tests/src/Unit/StateTransitionValidationTest.php
 ------ -------------------------------------------------------------------------
         Class PHPUnit_Framework_TestCase not found and could not be autoloaded.
 ------ -------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   tests/src/Unit/WorkbenchPreprocessTest.php
 ------ -------------------------------------------------------------------------
         Class PHPUnit_Framework_TestCase not found and could not be autoloaded.
 ------ -------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------
  Line   workbench_moderation.module
 ------ --------------------------------------------------------------------------------------------
  211    Fetching class constant class of deprecated class Drupal\node\Plugin\Action\PublishNode.
  214    Fetching class constant class of deprecated class Drupal\node\Plugin\Action\UnpublishNode.
 ------ --------------------------------------------------------------------------------------------

joy29, did you run the drupal-check tool locally?

larowlan’s picture

Issue tags: +DrupalSouth 2019
agentrickard’s picture

I wonder...can we just drop this module in Drupal 9? Do we have a clean upgrade path to core Content Moderation?

redzeuf’s picture

Status: Needs review » Needs work

Good point @agentrickard, I guess this module should be a minimal maintain for D9 just because of all the D8 websites that used it already and don't have the capacity to migrate to the core Content Moderation.

Second point, we should update the scope in the summary of this issue regarding the comment #7 #3042757-7: Drupal 9 Deprecated Code Report

larowlan’s picture

Version: 8.x-2.x-dev » 8.x-1.x-dev

As much as I would *love* to just not update WBM for Drupal 9, I think that presents a problem.

As a community, our positioning and narrative around Drupal 8 to 9 has been 'Drupal 9 will be a painless update'.

And that is the case if the modules you have on Drupal 8 are updated for Drupal 9 between now and June 2020.

The issue is, there are still more than 5000 D8 sites using WBM (see https://www.drupal.org/project/usage/workbench_moderation), and its most likely that nearly all of these are early adopters of D8.

Having been through the WBM » CM migration on some production sites, I can say from experience that it is not 'painless'. So if we say 'there will be no D9 compatible WBM release' it will be contradictory to the 'Drupal 9 will be a painless update' narrative we've been telling as a community.

So I think for that reason we should provide a D9 version, but we should then put the module into a 'no further development' state and aggressively mark feature requests and non-critical bugs as Closed (Won't fix)

I intend to try and work on this at DrupalSouth 2019 code sprint (later this month).

agentrickard’s picture

What would we need to do to make the transition 'painless'? Do we have a list?

Tolyan4ik’s picture

Status: Needs work » Needs review
FileSize
17.07 KB

Please check

mattbloomfield’s picture

Hi,

I've applied the patch in #13, but I'm trying to install on an instance of Drupal 9. I'm getting the following error in my php log:

PHP Exception InvalidArgumentException: "No converter has been registered for paramconverter.latest_revision" at /srv/bindings/991c5136570c4bcfab407f525abcd258/code/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php line 39

And in drush I get this error:

You have requested a non-existent service "entity.manager".

It does say entity_type.manager in the services file, so I don't know where the error could be.

larowlan’s picture

larowlan’s picture

Title: Drupal 9 Deprecated Code Report » [PP-1] Drupal 9 Deprecated Code Report
Status: Needs review » Postponed
agentrickard’s picture

I *really* don't think we should allow new installations in Drupal 9. Force new users to use Content Moderation.

Sam152’s picture

Status: Postponed » Needs review

Blocker is in.

larowlan’s picture

Title: [PP-1] Drupal 9 Deprecated Code Report » Drupal 9 Deprecated Code Report
larowlan’s picture

larowlan’s picture

larowlan’s picture

Status: Needs review » Needs work

The last submitted patch, 23: 3042757-23.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

Missed a comma in the annotation change

larowlan’s picture

larowlan’s picture

larowlan’s picture

larowlan’s picture

larowlan’s picture

munish.kumar’s picture

Status: Needs review » Needs work

Hi @larowlan
The latest patch returns with 317 coding standards messages. See: https://www.drupal.org/pift-ci-job/1719773

larowlan’s picture

Status: Needs work » Needs review

Thanks @munish.kumar - but it also notes

"✓ 10 fewer than branch result"

So we're getting better.

I'd prefer to keep this focussed on D9 functionality and open a separate issue to fix all of those once this is in.

munish.kumar’s picture

@larowlan, Thanks for the clarification. I agree with your thoughts, It would also help people to update to D9.

larowlan’s picture

Discussed this with @munish.kumar via slack, and he agreed with the above. Crediting him for his time to reply.

I opened #3150430: Fix coding standards for that.

Sam152’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -midcamp2019, -DrupalCampBelarus2019, -DrupalSouth 2019

LGTM, nothing worth blocking over, especially given the module is mostly on life support.

  1. +++ b/src/Entity/Handler/ModerationHandler.php
    @@ -35,10 +31,20 @@ class ModerationHandler implements ModerationHandlerInterface, EntityHandlerInte
    +    // When entities are syncing, content moderation should not force a new
    

    Reference CM not WBM, but hardly worth fixing.

  2. +++ b/src/Entity/Handler/ModerationHandler.php
    @@ -35,10 +31,20 @@ class ModerationHandler implements ModerationHandlerInterface, EntityHandlerInte
    +    if (!$entity->isSyncing()) {
    +      $entity->setNewRevision(TRUE);
    +      $entity->isDefaultRevision($default_revision);
    +    }
    ...
    +    if (($entity instanceof EntityPublishedInterface) && $entity->isPublished() !== $published_state) {
    

    Supporting syncing and EntityPublishedInterface seem like new features, which is interesting to add this late in the game, although hopefully not disruptive. Seems like it would be fine IMO, just pointing it out.

  3. +++ b/src/Entity/Handler/NodeModerationHandler.php
    @@ -50,19 +38,4 @@ class NodeModerationHandler extends ModerationHandler {
    -  protected function shouldModerate(ContentEntityInterface $entity) {
    -    // First condition is needed so you can add a translation.
    -    // Second condition is needed when you want to publish a translation.
    -    // Third condition is needed when you want to create a new draft for a published translation.
    -    return $entity->isDefaultTranslation() || $entity->moderation_state->entity->isPublishedState() || $entity->isPublished();
    -  }
    

    I don't really understand the implications of removing this check. I'm reading it as: WBM never supported moderating translations independently but now you can?

    Multilingual and pending revisions is very tricky, if it's purely as a new feature, it might be worth just leaving the restriction in there and directing people to upgrade to CM if they need translations.

    At least then, the bugs will be in one queue.

larowlan’s picture

Thanks @Sam152

1-3 were to get existing ModerationLocaleTests tests passing on D9, not trying to adding new features. I followed the lineage on content moderation in terms of what happened to the code that was in NodeModerationHandler, and the commits that changed - we already supported multilingual, but without those changes it was failing on D9. So something changed in either NodeForm or similar - so this is basically a backport from CM

  • larowlan committed b7199c7 on 8.x-1.x
    Issue #3042757 by larowlan, joy29, Tolyan4ik, agentrickard, Sam152,...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed
penyaskito’s picture

@larowlan

+++ b/src/ParamConverter/EntityRevisionConverter.php
@@ -71,11 +71,11 @@ class EntityRevisionConverter extends EntityConverter {
-      list($entity_type_id, $operation) = explode('.', $default);
-      if (!$this->entityManager->hasDefinition($entity_type_id)) {
+      [$entity_type_id, $operation] = explode('.', $default);
+      if (!$this->entityTypeManager->hasDefinition($entity_type_id)) {

This raises the requirement from PHP 7.0 to 7.1. Not sure if that was in purpose given that I assume this will still be low maintenance.

https://3v4l.org/aieRN

larowlan’s picture

@penyaskito argh yeah that's PHPStorm it auto changes that with my current language level settings.

Thoughts - 7.0 is long EOL and based on https://www.drupal.org/docs/system-requirements/php-requirements not recommended anymore.

I'm happy to wind it back if needs be, but realistically people need to be moving towards 7.3 at this point in time.

Status: Fixed » Closed (fixed)

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

joshua.boltz’s picture

I don't understand why this was marked as Fixed, because I've upgraded to the latest version of this module (which includes this patch to add D9 fixes), but when i re-run the Drupal-check, i still see these errors

./vendor/bin/drupal-check docroot/modules/contrib/workbench_moderation
 72/72 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -----------------------------------------------------------------------------------------
  Line   src/Entity/ModerationStateUninstallValidator.php
 ------ -----------------------------------------------------------------------------------------
  63     Call to deprecated method getFieldStorageDefinitions() of class
         Drupal\Core\Entity\Sql\SqlContentEntityStorage:
         in drupal:8.7.0 and is removed from drupal:9.0.0.
         Use \Drupal\Core\Entity\EntityFieldManagerInterface::getActiveFieldStorageDefinitions()
         instead.
 ------ -----------------------------------------------------------------------------------------

 ------ ----------------------------------------------------------------------
  Line   src/Form/PrepareUninstallForm.php
 ------ ----------------------------------------------------------------------
  114    Call to deprecated function drupal_set_message():
         in drupal:8.5.0 and is removed from drupal:9.0.0.
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
 ------ ----------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------
  Line   tests/src/Kernel/ViewsDataIntegrationTest.php
 ------ ---------------------------------------------------------------------------------------------------------
         Class Drupal\Tests\workbench_moderation\Kernel\ViewsDataTest was not found while trying to analyse it -
         autoloading is probably not configured properly.
         💡 Learn more at https://phpstan.org/user-guide/autoloading
 ------ ---------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------
  Line   workbench_moderation.install
 ------ --------------------------------------------------------------------------
  86     Call to deprecated function db_table_exists():
         in drupal:8.0.0 and is removed from drupal:9.0.0. Instead,
         get a database connection injected into your service from the container,
         get its schema driver, and call tableExists() on it. For example,
 ------ --------------------------------------------------------------------------


 [ERROR] Found 4 errors

So not sure how these were missed as part of this issue.

larowlan’s picture

@joshua.boltz other than the incorrect test namespace for the views test, none of those files exist in workbench moderation HEAD.

So I guess that means you've got a patch applied? That patch will need updating for D9 would be my guess.

I'll open an issue for the test namespace - #3170945: Class ViewsDataTest does not match filename ViewsDataIntegrationTest.php