Contrib.social

Book export routing slug {type} is not functional

Open on Drupal.org →
Created on 10 January 2021, about 4 years ago
Updated 21 July 2023, over 1 year ago

Problem/Motivation

The routing for book export currently has {type} as a slug. This slug is used to define the callback to create the document. In Drupal 7.x this made sense as one could add a function book_export_{type} to a module and it would be called. In Drupal 9.x the callback is however called within the BookExport class. There is only one such callback (BookExportHtml) specified in the class and it is not possible to add new functions to this class by other modules. Therefore, {type} as a slug is currently not functional.

This might even lead to an issue if someone would write another module to export the books to another format. It would be logical to use the path /book/export/new_format/{node}. When the new module name is alphabetically ordered after book it will overwrite the path. However, if the module name is alphabetically ordered before book the path will be claimed by book and a Page not found / Unknown format message is shown. Moreover, it seems wrong that a module claims a non-functional path.

Steps to reproduce

Two small modules showing a standard page can be made, one with a name alphabetically ordered before book and one with a name alphabetically ordered after book, e.g. abookexporttest claiming /book/export/atest/{node} and a module bookexporttest claiming /book/export/test/{node}. I've added these modules as an attachment.

When the path /book/export/atest/{node} is visited it shows "Page not found" with the remark "Unknown Format" as expected from the book module.
When the path /book/export/test/{node} is visited it shows the page from bookexporttest.

Proposed resolution

The code of bookExport contains a comment that the custom export functionality should be converted to serializer. I'm not sure how serializer exactly works and cannot find information on its implementation in this regard. But I suppose that if it's possible for a new module to register an encoder to serializer which is then dynamically called by book this would solve the problem.

Another solution would be to remove the slug:
In book.routing.yml set book.export path: to /book/export/html/{node}

In BookController.php the function bookExport could be adapted to the hard coded html, the check for unknown formats can then be removed:

  public function bookExport(NodeInterface $node) {
    $exported_book = $this->bookExport->bookExportHtml($node);
    return new Response($this->renderer->renderRoot($exported_book));
  }

Remaining tasks

1. Review the problem and suggest the best solution.
2. Write & review a patch.

User interface changes

None

API changes

Depends on the solution. I suppose using serializer might change the API.

Data model changes

None

Release notes snippet

🐛 Bug report
Status

Postponed

Version

11.0 🔥

Component
Book  →

Last updated 16 days ago

Created by

🇳🇱Netherlands cterveen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇳🇿New Zealand quietone

    This extension is being deprecated, see 🌱 [Meta] Tasks to deprecate Book module Active . It will be removed from core and moved to a contrib project, 📌 [11.x] [Meta] Tasks to remove Book Active .

    This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project → and the Extensions approved for removal → policies.

    This issue may be re-opened if it can be considered critical → , If unsure, re-open the issue and ask in a comment.

  • Status changed to Active 9 months ago
  • Comment 9 months ago →
    🇺🇸United States smustgrave

    Moving to contrib.

  • Status changed to Postponed: needs info 5 months ago
  • Comment 5 months ago →
    🇺🇸United States smustgrave

    Wonder if still an issue in 2.0.x for contrib?

  • Comment 4 months ago →
    🇺🇸United States wmfinck

    I was persuaded that this is a related issue, but as I wrote this, it might be an Adaptive Theme issue instead:

    In both Book 1.0.0 and in Book 2.0.0-alpha1 I am getting the following error whenever "Printer Friendly Version" is clicked: "The website encountered an unexpected error. Try again later."

    The information in the Drupal database log:

    Location https://example.com/book/export/html/5
    Referrer https://example.com/content/some-book-url
    Message Twig\Error\RuntimeError: The "filter" filter expects a sequence/mapping or "Traversable", got "integer". in Twig\Extension\CoreExtension::filter() (line 37 of /var/www/vhosts/example.com/site/web/themes/contrib/adaptivetheme/at_core/templates/layout/book-export-html.html.twig).

    But I just found that this is not an issue in Olivero. I will check/make an issue at Adaptive Theme.

  • Comment 4 months ago →
    🇺🇸United States smustgrave

    Thanks for checking!

  • Status changed to Closed: outdated 2 months ago
  • Comment 2 months ago →
    🇳🇱Netherlands cterveen

    I've just started looking into this module again and was curious if this issue was resolved. The problem wmfinck describes is different than
    the one described here and doesn't look related.

    I haven't tested it with version 2.0.0 alpha (can do if you wish), but from the code in book.routing.yml and BookController.php I get that this problem still exists.

    I also noted that Book PDF uses it's own route rather than /book/export/pdf/{node}, I assume due to similar issues as the one described here.

  • Comment 2 months ago →
    🇺🇸United States smustgrave

    If you test with 2.0.x and still notice it feel free to reopen.

  • Comment 2 months ago →
    🇳🇱Netherlands cterveen

    I've tested it in a new 11.0.5 Drupal Install with Book 2.0.0-alpha5.

    At first I couldn't replicate the bug, both /book/export/atest/1 and /book/export/test/1 worked fine.
    Then I installed my own module bookexportrtf and /book/export/atest/1 stopped working: "Unknown export format.", while /book/export/test/1 worked fine. This is what's in my original report.
    I uninstalled my module and both urls worked again.

    So there seems to be some strange interaction with my own module going on I'll have to investigate. But it looks like the Book Module on itself is working fine and the bug report can remain closed.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024