Problem/Motivation

As we converting more entity types to be publishable, in the sense of extending EntityPublishedInterface, the default Views wizard plugin could simplify the work needed by its subclasses by adding a default filter on the 'published' entity field.

Proposed resolution

Do it.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
FileSize
5.92 KB

It should be as easy as this patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2905000.patch, failed testing. View results

amateescu’s picture

Status: Needs work » Needs review
FileSize
5.94 KB
777 bytes

And this interdiff :)

Lendude’s picture

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

Nice! Love the unification.

Comment wizard has coverage for this: \Drupal\comment\Tests\Views\WizardTest
Media and media revision have coverage for this : \Drupal\Tests\media\FunctionalJavascript\MediaViewsWizardTest

Node revision has a test but no coverage for this: \Drupal\Tests\node\Functional\Views\NodeRevisionWizardTest
Node wizard doesn't have tests that I see.

So 'needs work' to get the node change covered

timmillwood’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
1.55 KB
7.53 KB

Implementing filter assertions in NodeRevisionWizardTest

amateescu’s picture

The test additions look good to me, now we need someone to RTBC this :)

Lendude’s picture

New test look perfect, but:

Node wizard doesn't have tests that I see.

Did somebody recheck this? Did I overlook them or are these still needed? Slightly modified version of \Drupal\Tests\node\Functional\Views\NodeRevisionWizardTest would probably be great as base coverage.

amateescu’s picture

@Lendude, the node wizard is tested in \Drupal\Tests\views\Functional\Wizard\NodeWizardTest ;)

Lendude’s picture

Status: Needs review » Reviewed & tested by the community

@amateescu ha! yeah like I said, I may have overlooked that. Silly me, not looking in the Views module for a test for something in the Node module :)

Existing coverage in \Drupal\Tests\views\Functional\Wizard\NodeWizardTest looks good. So this is now all covered.

larowlan’s picture

Issue summary: View changes
larowlan’s picture

Status: Reviewed & tested by the community » Needs review

Can we get a change notice here?

amateescu’s picture

larowlan’s picture

Thanks

Manuel Garcia’s picture

Status: Needs review » Reviewed & tested by the community

  • larowlan committed 32e4022 on 8.5.x
    Issue #2905000 by amateescu, timmillwood: Add a default filter on the '...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed as 32e4022 and pushed to 8.5.x.

Published change record

xjm’s picture

I missed this before but this is a really great change. Great work.

amateescu’s picture

Thank you :)

Status: Fixed » Closed (fixed)

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