when testing #3305971: Unexpected error on model export i also tried to re import the model. that caused another unexpected error unrelated to the one in the linked issue according to @jurgenhaas

The website encountered an unexpected error. Please try again later.
TypeError: Drupal\config\StorageReplaceDataWrapper::replaceData(): Argument #2 ($data) must be of type array, bool given, called in /var/www/html/web/modules/contrib/eca/modules/ui/src/Form/Import.php on line 325 in Drupal\config\StorageReplaceDataWrapper->replaceData() (line 198 of core/modules/config/src/StorageReplaceDataWrapper.php).
Drupal\config\StorageReplaceDataWrapper->replaceData('views.view.', ) (Line: 325)
Drupal\eca_ui\Form\Import->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 114)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 52)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 592)
Drupal\Core\Form\FormBuilder->processForm('eca_import', Array, Object) (Line: 320)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 564)
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}() (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 50)
Drupal\ban\BanMiddleware->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

i've added the model i tried to re import to the issue. as noted in the other issue the model is just a collection of all available events actions and conditions and not all fields are filled properly - the sole purpose was setting up a model to check its micro copy in context.

CommentFileSizeAuthor
#11 default.png460.46 KBrkoller
bpmn_io-process_z6jzc9k.tar-6.gz49.88 KBrkoller

Issue fork eca-3306003

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rkoller created an issue. See original summary.

jurgenhaas made their first commit to this issue’s fork.

jurgenhaas’s picture

Status: Active » Needs review

This is an interesting one because it's such an edge case that touches on so many different areas.

First of all, the problem is caused by the 2 action VIEWS: EXECUTE QUERY and VIEWS: EXPORT QUERY INTO FILE, both of which have no view selected. This now causes a series of follow-up issues that then result in the error above:

  • First issue is that the views actions do not insist in a view being selected. Those plugins make no sense ever without any selected view. So, that field should be declared required and when preparing the config form, the first view in the list should be pre-selected, so that those plugins can never be stored with an empty field.
  • The second issue then, that the model should have never been saved with the missing field being filled. With the approach above, this would have been prevented already. So, no action required here.
  • When calculating the dependencies while saving the model, we take the empty views id and declare a dependency on the config entity views.view. - of course that config entity does not exist. Should we recognize that when calculating the dependencies?
  • Finally, the exported model contains invalid data and that then breaks the import. As import is always dealing with unknown data, it needs to be careful with what it's doing with it. We already validate a lot, such an inconsistent config entity was never on our mind before. We're now checking for that and ignore such broken config entities and print an error message to the user, explaining which config entity was broken.

The MR !219 fixes the issue. @rkoller when trying this out, you could first try and import that broken model and the process should no longer fail with an exception. Instead it should print an error message.

Then, you could re-export your model after you have selected a view for the two action plugins. Then, re-importing it should work without an issue.

rkoller’s picture

Status: Needs review » Active

you are correct about the first part. after applying the patch and trying to reimport the model i get the following error now:

The contained config entity %name is invalid and got ignored.

i've then taken a look at the two tasks in question. each actually has a view selected in the select list, it is set to archive. archive is the default view that is assigned. problem is in a default drupal installation archive is set to disabled. that way it was unavailable as well. would it make sense to use a different default view for those tasks? also it would be a reasonable step to only list views that are enabled instead of enabled and disabled views?

i've enabled the archive view. i've then exported the model again. reimported the exported model again and i ran into the same error like above again:

The contained config entity %name is invalid and got ignored.

and the model isn't added to the model list. not sure if the issue has to be set back to needs work or if that is a way too fringe edge case that further steps arent necessary? just set it to active. but i think the default view for the two tasks might need a follow up?

jurgenhaas’s picture

Status: Active » Needs review

Oh, the string %name should have been replaced with the name of the broken config entity. I've fixed that.

Also, the list of available views is now limited to the enabled ones.

The fact that when you look into the property panel now, there is always a view selected, i.e. the first available one if not otherwise selected by the user. That's what happens when a field is declared "required".

Please give it another try and if the re-import still doesn't work, we should now see which config entity is still causing the problem.

rkoller’s picture

Status: Needs review » Needs work

hm cleared caches and also cleared the caches inside the browser window plus i've then reloaded the page. then exported the model another time and then reimported the exported file. the error states now:

The contained config entity views.view. is invalid and got ignored.

the view was still set to Archive. but that Archive view is enabled. i've also checked eca.ddev.site/archive and that archive view has actual content and does output something. therefor i wonder what is causing the error.

i've then changed the view to content for the two tasks, saved the model, exported, and reimported. that way the import worked. status message is with content as view then:

There are no changes to import.

it is an identical model. but just for clarity if the model is identical it doesnt get imported because it has the same model id than the existing model? because reimporting an existing model i would consider the same as clone and would have assumed the model would get cloned and you get identical entries?

jurgenhaas’s picture

OK, selecting a view for your two actions is necessary so that the model can get saved properly. So, I'd consider the export working now. And it seems that the import is working too.

To your question: importing a model is NOT cloning. It imports that model as is and if a model with the same ID already exists, it will replace that. It's like importing config with Drupal's CMI, we're using exactly the same mechanism, just limited to a model and its dependencies.

rkoller’s picture

oh i think i've been able to narrow down the problem:

1. create a new model
2. just add one task (views: execute query)
3. leave the default view to archive (don't touch the selectbox assuming the default value archive will be selected as your view)
4. save the model
5. export the model
6. reimport the model-> error.

1. create a new model
2. just add one task (views: execute query)
3. change the default value archive in the select box to something else eg media and then directly back to archive
4. save the model
5. export the model
6. reimport the model-> works.

i agree import and export seem to work properly now but the problem is with the setting of the default value for a view imho. without an input/interaction with the views select list the default isn't applied on save and remains empty instead?

ahhh thanks for clarification i've always assumed importing would create a new model. good to know.

jurgenhaas’s picture

Status: Needs work » Needs review
Issue tags: +backport

Perfect analysis, thanks @rkoller

This is where the javascript based forms in bpmn_io behave differently from the html forms that Drupal's form API builds. We have to fix that by explicitly setting the value for the drop down, if that field is required.

I've fixed that in the MR but please be aware, that only applies to actions that you add to models. Already available actions (or other elements) don't get updated by this fix. You will either have to manually select an item for your two existing actions or remove them from the model and re-add them - they will then have proper values.

If that's confirmed, I guess we can RTBC this one. I have also identified a few more minor issues and will open a couple more issues for them.

rkoller’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
460.46 KB

i've retested the steps described in #9. on the left without the patch on the right with the patch. (so it is tested with a completely newly created model minding the caveat you've mentioned @jurgenhaas)

export model screens for with and without the patch - with patch the default view gets exported

  • jurgenhaas committed 0e2936b on 1.1.x
    Issue #3306003 by jurgenhaas, rkoller: Unexpected error on model re...

  • jurgenhaas committed 7898495 on 1.0.x
    Issue #3306003 by jurgenhaas, rkoller: Unexpected error on model re...
jurgenhaas’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @rkoller for all the testing. Great improvement.

Committed and back-ported.

Status: Fixed » Closed (fixed)

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