Thumbnail

The bloated theme anti-pattern of GovCMS

Thumbnail

Si Hobbs

|

GovCMS SaaS has encouraged php hacks in the theme since the Acquia platform - nothing has really changed. It's time to really challenge the notion that theme-only GovCMS sites are a sustainable practice.

The most frustrating aspect of working with GovCMS SaaS is the arbitrary restriction on custom modules. Due to a limitation of the Acquia platform, we remain unable to implement useful hooks, write update scripts for the content model, and create nice reusable plugins that could benefit the wider community.

The myth of the safe theme

The restriction on custom modules in a SaaS site is arbitrary. It's practically meaningless. It started as a by-product of how the Acquia Cloud Site Factory (ACSF) platform works, which was the previous solution for GovCMS. ACSF was designed as a massive multi-site solution: a myriad of cookie cutter sites which lightly themed the underlying distribution. It was an ill-fit for GovCMS because government sites are rarely cookie-cutter. The way GovCMS survived on ACSF was by having an obnoxiously bloated Drupal 7 distribution. 

The upshot is that the theme-only "feature" of the platform become a requirement of the new platform; a like-for-like replacement. But along the way (I speculate), it became recognised as the clearest and most manageable way of controlling the risk surface of a SaaS site.

As great as that sounds, theme-only development actually encourages code that is worse than what you would find in an equivalent Drupal module. What you can do in 3 lines in a module, might take 50 lines in a theme. What you could do best practise in a module (following a simple article on Stack Overflow), you may end up doing as a hard-to-follow workaround in the theme (using a solution that took you days to write). 

Anyone can write terrible and dangerous PHP in a theme. It's PHP, you have access to most of the Drupal core APIs and utility classes, you can update the database, you can print user data, alter the user session, you can implement hooks on behalf of system or govcms_security module, etc. Let's not mince. It will be difficult to overlook as the platform seeks increased levels of certification.

So who cares?

Why do we really care about whether the code is in the theme or a module? Not security that's for sure. Security is an issue whenever you have PHP, you should be auditing all PHP, not assume your theme is safe and a module is not.

In my opinion, the problems we face with code in the theme are either long-term rot, or missed opportunity. When I put dodgy code in a theme then:

  1. I have no ability to blog, or otherwise guide other SaaS developers into how to do this properly.

  2. My solution is not generic and re-usable by GovCMS team in the distro, if they decided to go that path. (Whereas a new field formatter would be exquisitely self-contained.)

  3. I'm proliferating solutions which are not considered best practice.

  4. It's really hard to explain why it's not good practice.

  5. It is harder to teach how to sanitise data when you're not dealing with normal practical examples.

  6. It takes a lot longer to workaround limitations with theme code than it takes to use a best-practice well-known solution in a custom module.

  7. My patterns could break with no worning when using the Drupal API in unusual ways.

What are the rules anyway?

We need custom code in the theme (see the case study below) to get the job done, and it's not in anyone's interest to restrict our ability to do this, and it's difficult to regulate. What you are allowed to do in the theme would be best covered by a Good Boy licence - do what your mother would approve of. 

However when it comes to the grey area, the smelly code, the stuff your mum doesn't quite understand you're up to, what will that conversation look like with the client? It starts like this:

Hey Dept of Foo, I spent a couple of days working out how to deliver feature 123, but to be honest the solution feels dirty, uses obscure patterns, and a core developer told me it should be in a module.

Personally, I feel there is no positive outcome to be had with this conversation, as it's very subjective. My job as a consultant/developer is to to deliver features, and personally I try to deliver features in a way that puts the content model first. I ask myself the question: can I remove the entire theme and still have meaningful content model? But that's just me. Some developers don't give a binary bit about content-first site building.

And the worst case is that the client goes to GovCMS team and says "hey our vendor says they idealogically shouldn't deliver feature 123, but you said that generic.gov.au has it, please explain". It doesn't work out well that way, the vendor will accommodate the feature, or otherwise be marginalised as unfit to deliver.

Solutions

Of course the GovCMS team has fear of custom code, ACSF weaknesses, and massive patches they blindly applied in the past. Wouldn't it be nice to enforce themes to have no PHP at all? Unfortunately the backbone of Drupal theming remains the _preprocess hook functions, which allow a theme to prepare variables for templating. In a perfect world, GovCMS would offer no PHP hosting for SaaS sites.

GovCMS SaaS solutions all want to be cookie-cutter, but every site is a snowflake. Maybe we can MOG all the the departments into one, and force them to accept a one-stop shop of government website design?

None of this will happen. How can we help spread best practice in the SaaS world? How do we prevent vendors writing and proliferating undesireable code in the theme?

I've listed some of the things that may help the GovCMS team. This list goes beyond the scope of this article, some is related to my previous post about the upstream challenge, but ultimately a lot of this stuff is entwined.

  1. Engage vendors about the features they need in SaaS+
  2. Allow a custom module in SaaS, audit it (and then improve theme auditing)
  3. Have a distro module that implements useful hooks, and provides corresponding hooks for themes
  4. Get expertise to identify the difference between regrettable and benign changes
  5. Allow sites to starting using whitelisted (benign) patches

Addendum: Case study

To get technical, here is a hypothetical (and intentional grey area) example where one would work significant extra hours trying to find a best practice way to do something, only to end up blocked, and finally adding a bunch of awkward PHP into the theme.

The pattern of frustration has played out many times on my last SaaS project. 

Say I have created a custom block type, and thus a library of content blocks (this entity is provided by core's block_content module, as opposed to the block module which deals with layout). I created a Paragraph type that allows an editor to assign a bunch of these custom blocks into a content page. The upshot is that editors are editing these blocks in one central place - write once, use everywhere.

The technique is not that common, but if you are following the emergent patterns around layout builder, you may agree that having reusable  block_content is a very future-proof concept.

The problem is that theming block_content is a pain unless the block was added through Drupal's Block layout admin page. It intentionally has no way to template it or preprocess it. To quote Berdir, a core maintainer:

if you want to display something with views or references or ..., then don't use block_content but either a node type or custom entity type. 

(In other words, "custom block content" which has a flag reusable in the database, is not recommended for resuable custom block content, except in one 20 year old layout paradigm. *thinking face*.)

Ok. So what are some of my options:

  1. Rework the content model as node, and get a bunch of public URLs and other Node API interactions I don't want, and that I can't suppress, at least not without a module).
  2. If I had a module, write a custom entity, but I need to duplicate the behaviour of blocks and there will be no one to maintain this as layout builder comes into prominence.
  3. If I had a module, implement the hooks needed to add theme pre-processing and templating to the block_content type.
  4. If I had a module, write a field formatter for entity reference fields which reference blocks.

The ideal solutions are #3 or #4. The lowest impact solution on the code is #1, but it has the highest impact on the  content model and in many cases would be is practically impossible with production data and no ability to use update hooks on SaaS. 

What I ended up doing was intervening at the field level, in a way that both gets the job done and is essentially legitimate. It doesn't actually look that bad on the surface, and many vendors would rejoice.

/**
 * Implements template_preprocess_field().
 *
 * Untested psuedocode, may contain bugs and security vulnerabilities.
 * Presented for blogging purposes only.
 */
function MYTHEME_preprocess_field(&$variables) {
  $field_name = $element['#field_name'];
  $field_type = $variables['element']['#field_type'];

  if ('field_foobar' == $field_name && 'entity_reference_revisions' == $field_type) {
    // Iterate over the field items, load the block, prepare the variables for the field template.
    foreach ($variables['items'] as $key => $item) {
      $pg = $variables['element']['#object'];
      if ($block_reference = $pg->get('field_service')->get($key)->getValue()) {
        $block = BlockContent::load($block_reference['target_id']);
        if ($block && $block->id() == $block_reference['target_id']) {
          $variables['items'][$key]['url'] = $block->field_foobar_link[0]->getUrl()->toString();
          $variables['items'][$key]['summary'] = $block->field_foobar_summary->value;
          $variables['items'][$key]['icon'] = '<i class="' . Html::cleanCssIdentifier($block->field_foobar_icon->value) . '"></i>';
        }
      }
    }
  }
}

 

No I haven't told my mum.

Add new comment

The content of this field is kept private and will not be shown publicly.

Plain text

  • No HTML tags allowed.
  • Lines and paragraphs break automatically.
  • Web page addresses and email addresses turn into links automatically.

Comments

  • Allowed HTML tags: <em> <strong> <cite> <blockquote cite> <ul type> <ol start type> <li> <dl> <dt> <dd> <p>
  • Lines and paragraphs break automatically.
  • Web page addresses and email addresses turn into links automatically.
  • Use [gist:#####] where ##### is your gist number to embed the gist
    You may also include a specific file within a multi-file gist with [gist:####:my_file].

Spread the word