#2708601: Allow per-field diff configuration introduced a regression for fields where a plugin cannot be found when doing a diff between two revisions with certain field types. The attached patch appears to resolve it.

The specific field that failed was an og membership reference field.

The resulting exception looks like this:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "" plugin does not exist. in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 52 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).
Drupal\Core\Plugin\DefaultPluginManager->getDefinition(NULL) (Line: 16)
Drupal\Core\Plugin\Factory\ContainerFactory->createInstance(NULL, Array) (Line: 84)
Drupal\Component\Plugin\PluginManagerBase->createInstance(NULL, Array) (Line: 108)
Drupal\diff\DiffEntityParser->parseEntity(Object) (Line: 116)
Drupal\diff\EntityComparisonBase->compareRevisions(Object, Object) (Line: 84)
Drupal\diff\Controller\NodeRevisionController->compareNodeRevisions(Object, '179365', '180181', 'raw')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 139)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 98)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 77)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 628)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Comments

rfay created an issue. See original summary.

rfay’s picture

Status: Active » Needs review
StatusFileSize
new909 bytes

Patch appears to resolve this problem.

Status: Needs review » Needs work

The last submitted patch, 2: diff.no_plugin_found_2708601_31.patch, failed testing.

miro_dietiker’s picture

Does this only affect stale data after update?
Or are the plugins newly properly dealing with dependency and config uninstalled when a module with plugins is uninstalled?

johnchque’s picture

Status: Needs work » Needs review

Actually it makes sense, since there can be some fields without any plugin available. Should we add test coverage for it?

johnchque’s picture

Assigned: Unassigned » johnchque

Of course right? It is a bug.

miro_dietiker’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new1.69 KB
new2.57 KB

Tests added, test-only = interdiff. Discussed with @Berdir, removed support for changed field and path. :)

The last submitted patch, 8: regression_exception-2768911-8-test-only.patch, failed testing.

miro_dietiker’s picture

Status: Needs review » Needs work

Yeah this looks fine.

However, you state "tests a field without plugins". Not telling what field.
All the issue that you proof is implicitly covered.

Please make explicit statement about what is missing and what we are asserting (such as a comment about what the problem was where it failed when hitting Compare...)

johnchque’s picture

Status: Needs work » Needs review
StatusFileSize
new3.24 KB

True, added better comments. :)

johnchque’s picture

StatusFileSize
new1.22 KB

Forgot the interdiff. :)

johnchque’s picture

Sorry, uploaded the wrong files.

miro_dietiker’s picture

Status: Needs review » Fixed
Issue tags: -Needs tests

OK Committed.

Please don't forget that the method docblock is the location where you need to describe the scenario. I extended it pointing at the specific case of the changed field without a plugin.

johnchque’s picture

Assigned: johnchque » Unassigned

Will take note for the next time, thank you. :)

Status: Fixed » Closed (fixed)

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