The Deploy UI throws some exceptions when exporting revision-enabled entity types with the ViewsAggregator as that one does not even return the revision id property.

The attached patch resolves that issue by adding some checks in place but it should eventually probably be fixed by fixing ViewsAggregator.

I might be missing something though... (No spare time atm. to dig deeper :/).

CommentFileSizeAuthor
#16 deploy-views_missing_revision_id-2196109-16.patch2.52 KBfmizzell
#8 deploy-views_missing_revision_id-2196109-8.patch2.85 KBgrasmash
#7 deploy-views_missing_revision_id-2196109-7.patch2.39 KBgrasmash
#3 deploy-views_missing_revision_id-2196109-2.patch1.38 KBtimaholt
#1 2196109-1.patch1.39 KBfubhy

Comments

fubhy’s picture

fubhy commented
Status: Active » Needs review
StatusFileSize
new 2196109-1.patch1.39 KB
dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented
Status: Needs review » Fixed

Thanks fubhy! :) Comitted to 7.x-2.x!

timaholt’s picture

timaholt commented
StatusFileSize
new deploy-views_missing_revision_id-2196109-2.patch1.38 KB

Just to reiterate the first patch is definitely a band-aid solution. Here's a patch to actually fix the views aggregator to provide the revisions from the aggregator.

timaholt’s picture

timaholt commented
Status: Fixed » Needs review
fubhy’s picture

fubhy commented

Yep, totally a band-aid solution. Had no time to dig deeper or provide a full fix. Thanks for jumping in!

+++ b/modules/deploy_aggregator_views/plugins/DeployAggregatorViews.inc
@@ -42,7 +42,11 @@ class DeployAggregatorViews extends DeployAggregatorBase {
+      $revision_key = $entity_info['entity keys']['revision'];

That brings the notices back if I am not mistaken. We need to check if revisions are actually supported. 'revision' entity key might not even be set.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented
Status: Needs review » Needs work

I did a blind commit with Tim's patch in #3 (I'm not in a position to test it right now) so I will leave this issue open so we can iterate on it and potentially fix the notices that may arise from missing revision keys.

grasmash’s picture

grasmash commented
Status: Needs work » Needs review
StatusFileSize
new deploy-views_missing_revision_id-2196109-7.patch2.39 KB

I've refactored Tim's changes by moving a couple of lines out side of the loop, and accommodating the edge case where the entity type doesn't support revisions. I've also rolled changes from #2204101: Pass contextual filter values to Views Aggregator into this patch, given that both issues modify the same hunk and cannot be applied separately--sorry to conflate the issues.

grasmash’s picture

grasmash commented
StatusFileSize
new deploy-views_missing_revision_id-2196109-8.patch2.85 KB

Forgot the form element.

timaholt’s picture

timaholt commented

Matt thanks for adding/refactoring. I will take a look at this patch. Also @fuhby good catch, but looks like matt beat me to fixing that.

hnln’s picture

hnln commented

This patch causes fatal errors on feature page if you use a view for entities without revision (I'm using it for eck entities). So revision integration should check if the is actually revision info.

fmizzell’s picture

fmizzell commented

@HnLn could you be a little more specific on how you are getting to the fatal error? Here is what I did trying to reproduce your problem:

  1. Created and entity type and bundle with eck.
  2. Added a text field.
  3. Created 2 pieces of content.
  4. Created a View to display the content.
  5. Created a deployment plan and associated the created view with the plan.
  6. Created a feature with all of the things I created (entity_type, bundle, fields, deployment plan, etc).
  7. Enabled the feature (No issues navigating through features and deploy UIs).
  8. Created a new sandbox, and enabled the feature there (No issues navigating through features and deploy UIs).

Maybe I am completely misunderstanding your problem, but a little more info will be helpful :)

fmizzell’s picture

fmizzell commented
StatusFileSize
new deploy-views_missing_revision_id-2196109-9.patch2.52 KB

This patch fixes a warning occurring when the views aggregator is being configured during a plan's creation.

Anonymous’s picture

Anonymous (not verified) commented

I applied the patch in #8 and it works for me.

What I did: created some file entities through the media module, add those to a feature, deleted the files and reverted the feature. Files were back in place as expected.

EDIT: Some weird stuff happened because we both posted at the same time here, I did not delete any patches like the post claims.

hnln’s picture

hnln commented

@fmizzell

Tnx for following up on this, it might be my local setup, it's rather complex content (lots of entity references several levels deep).

If I switch from deploy stable to the latest dev I get this error when I do a drush cc all or go to the features page:
PHP Fatal error: Cannot access empty property in sites/all/modules/contrib/deploy/modules/deploy_aggregator_views/plugins/DeployAggregatorViews.inc on line 49

If I change line 49 from
$entities[] = array('type' => $entity_type, 'id' => $entity_id, 'revision_id' => $entity[$entity_id]->$revision_key)
to
$entities[] = array('type' => $entity_type, 'id' => $entity_id);

The errors go away. It looks like an extra check is needed here to see if revision id is available ?

hnln’s picture

hnln commented

OK, I did miss that there were patches available :-) So my comment is mainly to point out that the current dev version is broken for entities without revisions. Applying the 12/13 patch also fixes the issue.

fmizzell’s picture

fmizzell commented
StatusFileSize
new deploy-views_missing_revision_id-2196109-16.patch2.52 KB

The same patch from 12 without style issues.

  • dixon_ committed 17f45db on autoplan-session 
    Issue #2196109 by fubhy: Missing revision id when using ViewsAggregator.
  • dixon_ committed 3ecb6b5 on autoplan-session authored by timaholt
    Issue #2196109 by timaholt, fubhy: Missing revision id when using...

  • dixon_ committed 17f45db on 8.x-1.x 
    Issue #2196109 by fubhy: Missing revision id when using ViewsAggregator.
  • dixon_ committed 3ecb6b5 on 8.x-1.x authored by timaholt
    Issue #2196109 by timaholt, fubhy: Missing revision id when using...

  • dixon_ committed 17f45db on 7.x-3.x 
    Issue #2196109 by fubhy: Missing revision id when using ViewsAggregator.
  • dixon_ committed 3ecb6b5 on 7.x-3.x authored by timaholt
    Issue #2196109 by timaholt, fubhy: Missing revision id when using...
john-m-adams’s picture

john-m-adams commented

It appears that there has been a recent commit on this issue, so I wanted to make sure that patch from #16 is still relevant/required for entities without revisions.

I am receiving the same fatal error as HnLn reported in #14, on 7.x-3.0-alpha1

pawel_r’s picture

pawel_r commented

Patch from #16 worked for me.