Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thoughts (in code) on the "Recent content" view #4

Open
wants to merge 3 commits into
base: 8.x-1.x
Choose a base branch
from

Conversation

becw
Copy link

@becw becw commented Oct 27, 2016

I was looking at #2733187 Unpublished content is not shown the other day and I started playing with the all recent content view a bit.

It's relatively easy for users to be in a situation where they don't have view access to content, but they can edit the content: they may have all of the permissions for a specific content type, but only have the "View own unpublished" content permission. Core doesn't provide a "View all unpublished" or a "View unpublished $type content" permission.

The changes I made are:

  • Displays all content (per the name of the view!)
  • Uses the same columns for the block and page views
  • Uses "Status: Published/Unpublished" instead of "Published: Yes/No" (to match the core content listing)
  • Adds view and delete links to the "Actions" column (lined up so it's easy to scan for what access you have)
  • Displays content titles as plain text when the user does not have view access (again, easy to scan)

Before:
screen shot 2016-10-26 at 6 47 57 pm

After:
screen shot 2016-10-26 at 7 36 16 pm

The problem here is that users get content in this listing that they can neither view nor edit--meaning perhaps it should be hidden all together. Would it make sense to add a views filter that emulates the core "Published status or admin user" filter, but also checks the user's permissions and see which types of nodes they have "edit any" access to? This would work for the base case, but I'm afraid that we run into the limitations of core node permissions right about now.

Also, and this might veer totally off track: would it make sense for Workbench to provide per-node-type "view unpublished $type content" permissions?

- Displays all content
- Uses the same columns in the block and page views
- Adds view and delete links to the "Actions" column
- Displays content titles as plain text when the user does not have view access

It's relatively easy for users to be in a situation where they don't have view access to content, but they can edit the content: they may have all of the permissions for a specific content type, but only have the "View own unpublished" content permission.
@agentrickard
Copy link
Owner

In D7, that permission was provided by Workbench Moderation.

In Drupal 8.2, it is provided by Content Moderation, and I think our path forward it to integrate with that module. (The identical permission is provided by Workbench Moderation.)

If we want to go with View unpublished $type, I can see that, but otherwise we would leverage core. My feeling is that Workbench proper is just a UI, and should leave the permissions to other systems.

@agentrickard
Copy link
Owner

See also https://www.drupal.org/node/273595

@becw
Copy link
Author

becw commented Oct 27, 2016

Ok, cool, I'm on board with that.

What do you think about removing the "Published status or admin user" filter from the "All recent content" Workbench view?

@agentrickard
Copy link
Owner

agentrickard commented Oct 27, 2016

Well, we're technically not controlling View access, so there isn't a security issue here except for unpublished nodes. Perhaps that means we add a "security" warning to the Workbench permissions that indicates it will allow users to view unpublished lists.

If we move the actions (edit | delete) to a column then it should be fine.

I don't know that we need a "view" action there, though.

@becw
Copy link
Author

becw commented Oct 27, 2016

I have removed the view link from the listing, and added description text to the "Access My Workbench" permission.

screen shot 2016-10-27 at 5 25 31 pm

@becw becw force-pushed the recent-content-refinements branch from 3b78185 to 5c86646 Compare October 27, 2016 22:35
@agentrickard
Copy link
Owner

Odd. I'm getting the following error on the All Content page view (but not in the Views preview).

Using core 8.2.x (dev).

Notice: Undefined index: published in Drupal\views\Plugin\views\filter\FilterPluginBase->acceptExposedInput() (line 1382 of core/modules/views/src/Plugin/views/filter/FilterPluginBase.php).

Drupal\views\Plugin\views\filter\FilterPluginBase->acceptExposedInput(Array) (Line: 157)
Drupal\views\Form\ViewsExposedForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 585)
Drupal\Core\Form\FormBuilder->processForm('views_exposed_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('\Drupal\views\Form\ViewsExposedForm', Object) (Line: 135)
Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase->renderExposedForm() (Line: 1225)
Drupal\views\ViewExecutable->build() (Line: 351)
Drupal\views\Plugin\views\display\PathPluginBase->execute() (Line: 168)
Drupal\views\Plugin\views\display\Page->execute() (Line: 1617)
Drupal\views\ViewExecutable->executeDisplay('page_1', Array) (Line: 78)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func(Array, Array) (Line: 376)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 226)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 227)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 144)
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: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
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: 652)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants