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

Add a system of page-less nodes #100

Closed
brantwynn opened this Issue Sep 25, 2013 · 115 comments

Comments

Projects
None yet
@brantwynn
Copy link

brantwynn commented Sep 25, 2013

Since we will soon have relegated blocks to being an admin-only text-only layout element that's saved into config, we are going to need a content-based alternative that is a fieldable entity!

Big picture, what we want are page-less nodes.

We have several examples of entities that come close in Drupal contrib that we can compare/contrast and decide if we want to start from any of them, or would prefer to create our own.


Use Cases

  1. Fieldable and typable block content:
    I often need to create a type of content that will only be viewed as a Block.
    Example: an Ad block that consists of an image field, a link field, an (optional) text field, and can be placed into a layout.
    Solutions: Bean or entity construction kit

  2. Content without a "page"
    I often need to create a type of content that will only be seen in a view.
    Example: a "Slider item" that consists of an image field, a text field, an optional description field and an optional link field.
    Solutions: entity construction kit or Rabbit Hole module

  3. Content as part of other content
    I often need to create a type of content that will only be seen as part of another piece of content.
    Example: Example: media-rich content. Sometimes content editors want to insert some media items between text paragraphs: sliders, photo galleries, maps, video, etc.
    Solution: paragraphs or entity construction kit + inline entity form or field collection or multifield

4) Grouping fields together (not a great fit for pageless nodes)
I often need to group my fields together, so that when I add another "field" I'm actually adding another "Set of X fields".
Example: A "contact person" may consist of a name field, email field, and phone field.
Solutions: field collection or multifield


PR by @serundeputy: backdrop/backdrop#2049
New PR, rerolled and tweaked by @herbdool: backdrop/backdrop#2286

@quicksketch

This comment has been minimized.

Copy link
Member

quicksketch commented Sep 25, 2013

Thanks for opening this issue @brantwynn! I don't think there's an issue yet for this, so this will probably become the official request.

I think for starters we might want to replace hook_block_*() hooks with a more typical hook_block_info() structure that specifies either callbacks (like hook_image_effect_info() or hook_filter_info()) or move to pointing at classes (like hook_views_info()). An info hook that points at a class effectively is the same thing as a D8 "plugin", since a plugin is basically nothing but a class and a discovery mechanism. If we forgo the automatic discovery and just use info hooks like traditional Drupal, we get the consolidated functionality of plugins without introducing a new concept into the system. Anyway, all that might be a precursor to this request. Since all custom blocks that are created would essentially be exposed through this hook, just like block module's implementation of its own hook today in block_block_info().

@jenlampton

This comment has been minimized.

Copy link
Member

jenlampton commented Jun 11, 2015

Wow, it's been a while since we looked at this issue. I, for one, would love blocks to be entities and for them to be fieldable in core. However, I don't think this can a happen in 1.x anymore.

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Jun 12, 2015

What if ...

  1. We add another optional view mode to nodes: Block
  2. We add another checkbox to the publishig options for content types that have the Block view mode active: [ ] Available as block
  3. Nodes with the expose checkbox show up as block, in the configured view mode.

Is this "oversimpling" the issue?

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Jun 12, 2015

Here's a proof-of-concept that exposes a node as a block, if checked in the publishing options.

https://github.com/backdrop/backdrop/compare/1.x...MrHaroldA:block_nodes?expand=1

It lacks a lot of things; but it's basically a "fieldable block system" of some kind ... ;)

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Jun 15, 2015

It would be cool if someone could check the architectural changes of my patch, and tell me if it's viable or not.

I expected this to be difficult to achieve, but my patch is really straight-forward ...

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Dec 16, 2015

@jenlampton: I, for one, would love blocks to be entities and for them to be fieldable in core.

I've been thinking a lot about this, and I have not been able to come up with many cases where it would make sense to add fields to blocks. Pretty much the only one would be a field to hold images that are used in the block.

Do folks have other examples where'd they add fields to blocks? And would these fields just be added to all the blocks on the system, or do we have to implement the concept of block types?

This was referenced Dec 17, 2015

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Dec 17, 2015

A reply from @jenlampton with examples of fieldable blocks that people might create:

ex: type: slide - a home pageslide that needs fields (video, image, text, link) and is only accessed via a view
ex: type: ad - an Ad needs fields (image, link) but is placed individually in a layout
ex: type: administrator - this item appears only within various lists of administrators. This content will not be associated with a user account, but needs fields (name, address, phone number) as well as categorization.

This will obviously not only require us to convert to the existing custom blocks to entities, but also provide support for different types of blocks.

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Dec 17, 2015

What about my "node as block" idea?

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Dec 17, 2015

I see where @jenlampton is coming from with the fieldable block types above, as I've created my share of slide and ad content types that didn't necessarily need to have their own public pages and all the bells and whistles that come with nodes, such as author and comments.

However, I very rarely needed to place these elements directly into the page as individual blocks, and instead usually have a view that displays either a list of these items or a single random item. In fact, it would be kinda overwhelming if a new block was created for every slide or ad that is added over the life of the site.

Therefore, while I'm on board with the idea of creating a new entity for creating fieldable types of content that don't need their own page, do we really want to call them blocks?

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Dec 17, 2015

@MrHaroldA: What about my "node as block" idea?

I like it this idea. Perhaps, just like we are creating a block that allows you to place individual fields into a layout in #1100, we could create another block type that allows you to place a whole node into the region, which has nid and display mode as settings. That might be simpler than defining a unique block for each node that you want to be able to use in a layout.

@klonos

This comment has been minimized.

Copy link
Member

klonos commented Dec 18, 2015

What about my "node as block" idea?

@MrHaroldA: do you have an issue filed for this already? Because the closest we have to that AFAIK is #1076.

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Dec 19, 2015

Not really, but I did make a proof of concept right here: https://github.com/backdrop/backdrop/compare/1.x...MrHaroldA:block_nodes?expand=1

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Dec 22, 2015

Hey folks, I've been thinking about the idea of fieldable blocks, and it seems like the big requirements we have been asking for are:

  • they are an entity
  • they don't have all the extras that are baked into nodes
  • you can create different types
  • you can add arbitrary fields
  • you can list them using views
  • you can add them individually to layouts

However, based on that, it seems like we already have a drupal entity that does all of those things (except for the backdrop-specific layout integration), and that is field collections.

For those who are asking for fieldable blocks (like @jenlampton), are there any features that you are looking for that are not supported by field collections? What if we were to take the existing implementation of field collections, ported it to backdrop, and perhaps changed the name to describe how you'd like to use them?

@MrHaroldA

This comment has been minimized.

Copy link
Member

MrHaroldA commented Dec 22, 2015

Field collections have way more features than we need, is painfully slow on nested collections, and has (or used to have?) big problems with revisions. I also dislike the default edit and delete links on the display within a node, but those can be disabled.

For example: each field collection type has it's own page callbacks, including view, edit and delete.

@jenlampton jenlampton changed the title Add fieldable, reusable blocks system Add a system of page-less nodes Jan 4, 2016

@jenlampton jenlampton modified the milestones: 2.x-future, 1.x-future Jan 4, 2016

@jenlampton

This comment has been minimized.

Copy link
Member

jenlampton commented Jan 4, 2016

I'm also in agreement about field collection being bloated and (more importantly) really slow. I'd love to see something architecturally cleaner than that, something more like a page-less node.

@serundeputy

This comment has been minimized.

Copy link
Member

serundeputy commented Jan 4, 2016

On the last project I worked on we made extensive use of the paragraphs module.

Which is basically a fieldable entity that is not a node. You can attach a paragraph bundle to a node or use it in a view, but it is not a block.

Just throwing that out there. I don't exactly understand all the points of view here (that doesn't make them bad), but paragraphs was a nice solution for me for some of the things discussed here.

#2¢geoff.

@mikemccaffrey

This comment has been minimized.

Copy link

mikemccaffrey commented Jan 5, 2016

@serundeputy: On the last project I worked on we made extensive use of the paragraphs module.

Good call. That project seems very pertinent to what we are looking for, and we should examine the architecture of that module closely, even if we don't do a direct port or end up building things in the exact same way.

@kreynen

This comment has been minimized.

Copy link

kreynen commented Aug 31, 2018

Are you going to change the node_type_get_types function in https://github.com/backdrop/backdrop/blob/1.x/core/modules/node/node.module#L380 so that every module won't have to get all the types and filter out the hidden/non-hidden options and add support to Views to be able to select all nodes of any content of either type?

@kreynen

This comment has been minimized.

Copy link

kreynen commented Aug 31, 2018

I'm trying to configure this how we'd actually use it. Not being able to filter views by types is a problem, but simply creating the number of bean types we currently have as pageless node types quickly impacts the usability of the core UI.

Currently node types and bean/block types are separated in UI.

screen shot 2018-08-31 at 2 07 30 pm

screen shot 2018-08-31 at 2 07 17 pm

While I understand that having this many content and block types is unlikely to be a usecase 80% of the Backdrop userbase shares, larger organizations are going to need additional functionality in core and views to be able to alter the UI into a usable state.

screen shot 2018-08-31 at 2 06 48 pm

@herbdool

This comment has been minimized.

Copy link

herbdool commented Aug 31, 2018

There was no discussion about altering node_type_get_types() but perhaps at minimum we can add a views filter to expose the hidden flag. I can't recall where else we've exposed config settings to views but seems like a good idea in this case.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Aug 31, 2018

@kreynen since you expressed interest in the views filter would you be able to produce some sample code for me to add to the PR? I'm not sure if I have time to add it before feature freeze (currently only have small bits of time here and there and not likely to have much time on this Canadian long weekend.

@kreynen

This comment has been minimized.

Copy link

kreynen commented Aug 31, 2018

We have a long weekend in the US too and @kevincrafts (our UI/UX manager) is off next week, but I'll see what we can do.

@olafgrabienski

This comment has been minimized.

Copy link

olafgrabienski commented Aug 31, 2018

@herbdool Did another quick sandbox review. Permission works now, nice! (After changing the permission, the setting applies however not immediately but only after clearing the "Page and else" cache.)

Regarding the terminology, I like 'path' instead of 'page' to avoid confusion with the Page content type. There is of course room for improvement, but I'd suggest to handle it in follow-up issues.

@alexfinnarn

This comment has been minimized.

Copy link

alexfinnarn commented Aug 31, 2018

<?php

/**
 * @file
 * Definition of views_handler_filter_node_hidden_path.
 */

/**
 * Filter by content type's hidden path setting.
 *
 * @ingroup views_filter_handlers
 */
class views_handler_filter_node_hidden_path extends views_handler_filter {
  function admin_summary() { }
  function operator_form(&$form, &$form_state) { }
  function can_expose() { return FALSE; }

  function query() {
    $table = $this->ensure_my_table();
    $node_types = config_get_names_with_prefix('node.type.');

    $enabled_types = array_filter($node_types, function($val) {
      return config_get($val, 'settings.hidden_path');
    });

    $enabled_types_names = array_map(function($val) {
      return str_replace('node.type.', '', $val);
    }, $enabled_types);
    
    $this->query->add_where_expression($this->options['group'], "$table.type IN (:types)", array(':types' => $enabled_types_names));
  }
}

By adding that views filter handler, an entry into node_views_data() and the autoload of the handler file, I was able to filter based on the hidden path setting.

The code could be cleaned up a little, and I took the example from views_handler_filter_node_status.inc.

@quicksketch

This comment has been minimized.

Copy link
Member

quicksketch commented Sep 1, 2018

I posted a short review to backdrop/backdrop#2286 (review). Code-wise, this looks good.

I'm not sure I like hidden path any more than pageless but it is more technically accurate. I'd prefer that for internal storage even if we decide to change the user-facing label and description for the option later.

Can we lop the word "display" off the permission name and change it from "view hidden path display" to "view hidden paths" (similar to "view revisions")? The word "display" gets conflated with display modes in my mind.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

@quicksketch I've updated the permission wording.

@alexfinnarn thank you! I've gotten pretty close based on the class you provided. But I might be doing something wrong in node_views_data. I can get the filters and field to show up but the options don't show in the filter.

Here's my working copy in node_views_data():

// hidden path
  $data['node']['hidden_path'] = array(
    'title' => t('Hidden path'),
    'help' => t('Whether or not the content has a hidden path.'),
    'filter' => array(
      'handler' => 'views_handler_filter_node_hidden_path',
    ),
    'sort' => array(
      'handler' => 'views_handler_sort',
    ),
  );

Not sure if I can get this figured in time for code freeze. Hopefully but any help you can lend is much appreciated.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

@alexfinnarn I updated my previous comment. It works for me but the options don't show up in the filter (no yes/no option). Not clear yet on how to get it to appear.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

I'm wondering if at this point we can merge the PR as is and open a new issue for the view filter, since it doesn't block the main PR.

@docwilmot

This comment has been minimized.

Copy link
Contributor

docwilmot commented Sep 1, 2018

@herbdool

    'filter' => array(
      'handler' => 'views_handler_filter_node_hidden_path',
      'label' => t('Hidden'),
      'type' => 'yes-no',               <<<<<-------------------- Add
    ),

and

class views_handler_filter_node_hidden_path extends views_handler_filter_boolean_operator <<<<<---

Then in the query in the filter handler,

if ($this->value) {   <<<<<<<--------- the user chose "Yes"

// Modify the query as appropriate

The code in that query gives me a headache with all the closures and $vals.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

Thanks @docwilmot I'll give it a try.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

Okay, ready for an other testing. I've now got a functioning views filter for including/excluding content with hidden paths with help from @alexfinnarn and @docwilmot.

cc @olafgrabienski

@olafgrabienski

This comment has been minimized.

Copy link

olafgrabienski commented Sep 1, 2018

@herbdool Here my quick sandbox test results:

(1) From the help text at the content type:

Users who have the "view hidden path" permission will still be able ...

Did you change the capitalized "View" to "view" on purpose? In my opinion, the capitalized version is more convenient.

(2) I wanted to add and configure the Views filter in the "Administer content" view and saw that it was already there and enabled ("Paths are hidden: Yes"), and it works! (I guess someone else had enabled the filter before. Can you double-check that it's not enabled by default?)

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 1, 2018

@olafgrabienski 1) nope it wasn't on purpose. I can change it. 2) Actually I added it. I wanted to double-check on a different website.

@quicksketch

This comment has been minimized.

Copy link
Member

quicksketch commented Sep 2, 2018

One new issue with the views handler: backdrop/backdrop#2286 (review)

My request from before was awkwardly made, so it didn't get conveyed quite:

Can we lop the word "display" off the permission name and change it from "view hidden path display" to "view hidden paths" (similar to "view revisions")?

@herbdool the new permission name is now view hidden path (singular). Can you make that plural; view hidden paths? Other places like on the node type where the setting is just called hidden_path look good to me.

@herbdool

This comment has been minimized.

Copy link

herbdool commented Sep 2, 2018

@olafgrabienski one last review? The main difference is the slight change in permission wording and a fix to the tests.

@olafgrabienski

This comment has been minimized.

Copy link

olafgrabienski commented Sep 2, 2018

@herbdool - done! Have tested the sandbox site regarding

  • wording √
  • hidden path setting vs. permission to view hidden path √
  • Views filter √
  • DBlog (no errors) √

So, RTBC in my opinion!

(I've also added the 1.11.0 milestone.)

@quicksketch

This comment has been minimized.

Copy link
Member

quicksketch commented Sep 2, 2018

Thanks @herbdool and @olafgrabienski! This looks great! I've merged backdrop/backdrop#2286 into 1.x for 1.11.0. Issue credit in the commit, as there are a lot of people to mention (backdrop/backdrop@e48317d).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment