unlcms / project-herbie

Drupal 10 implementation at the University of Nebraska–Lincoln
https://cms.unl.edu
GNU General Public License v2.0
5 stars 6 forks source link

Person content type #66

Closed ericras closed 4 years ago

ericras commented 4 years ago

Work can be migrated from UNL-CMS-2

ericras commented 4 years ago

23

ericras commented 4 years ago

1) Adds external_entities and a custom unl module for directory access 2) Adds unl_five subtheme to house templates specific to this distribution/project Config 3) Creates unl_directory_entry external entity and fields scoped with eeunldir 4) Creates content type person and fields scoped nperson

macburgee1 commented 4 years ago

External Entities Patch

Due to a change introduced in D8.7, External Entities needs a critical patch applied. Let's review that over on Drupal.org and apply it in this pull request: Override ContentEntityStorageBase::cleanIds to prevent fatal error introduced in D8.7. This patch replaces the custom patch included in this pull request.

Caching

Currently caching is disabled for external entities. I think we should consider setting it to at least a modest time period for UNL directory - 30 minutes? 60 minutes?

Page Title

Currently, we're using CSS to hide the page title, which is printed in the hero region. There's a pull request on unl_five that we can take advantage of: https://github.com/unlcms/unl_five/pull/10. It adds a 'unl_no_page_title' variable to the page template, which conditionally prints the page title in the hero region.

To take advantage of it unl_five_herbie, we'd need to add a unl_five_herbie.theme file with a hook_preprocess_page() function:

<?php
/**
 * Implements template_preprocess_page().
 */
function unl_five_herbie_preprocess_page(&$variables) {
  // Get current route.
  $current_route = \Drupal::routeMatch();

  // Preprocess nodes.
  $node = $current_route->getParameters()->get('node');
  if ($node) {
    $bundle = $node->bundle();

    switch ($bundle) {
      case ('person'):
        $variables['unl_no_page_title'] = TRUE;
        break;
    }
  }
}
?>

We can then remove the following from person.css:

body.page-node-type-person #dcf-page-title {
  display: none;
}

Schema

Configuration Inspector finds missing schema for the following:

external_entities.external_entity_type.unl_directory_entry.storage_client_config

It appears that external_entities_unl_directory doesn't define its schema. I have created an issue for that project: https://github.com/unlcms/external_entities_unldirectory/issues/1

Menu Configuration

Users are able to add 'Person' content to the Main menu. Is this by design?

macburgee1 commented 4 years ago

As part of this pull request, let's patch with Create database schema when external entity types are created. This will correct a bug in the module that I ran across during testing. It would be helpful to review the patch on Drupal.org to help get it committed.

macburgee1 commented 4 years ago

Update on External Entities patches:

Create database schema when external entity types are created

Override ContentEntityStorageBase::cleanIds to prevent fatal error introduced in D8.7

ericras commented 4 years ago

Everything above should be addressed now.

Regarding menu placement: yes, a Person node may be added to the menu. As an example: a professor's bio page (as a Person node) on a lab site.

macburgee1 commented 4 years ago

These changes all look good. Since I reviewed this pull request, we've had some new code committed to master (that has been merged here, too).

At this point, I think we just need to address the following: