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

Feature Request: Include chapter name with URL path for pages #497

Open
levest28 opened this issue Sep 6, 2017 · 10 comments
Open

Feature Request: Include chapter name with URL path for pages #497

levest28 opened this issue Sep 6, 2017 · 10 comments

Comments

@levest28
Copy link

levest28 commented Sep 6, 2017

For Feature Requests

Desired Feature: Option to include chapter in URL

For example:

Current URL for Chapter:
https://domain.ca/books/{bookname}/chapter/{chaptername}

Current URL for Page:
https://domain.ca/books/{bookname}/page/{pagename}

Desired URL for Page:
https://domain.ca/books/{bookname}/chapter/{chaptername}/page/{pagename}

Issue:
In current setup, if I have Chapter 1 with a page "About", Chapter 2 also with "About" page is named differently.

It would be better if both chapter can refer to the page name as "About", but include the chapter name in the URL.

For documentation purpose, if we have 100+ chapters, each with an "About" page, it looks pretty messy.

@ssddanbrown
Copy link
Member

Hi @levest28, Thanks for opening this issue/feature request.

Reasons For

  • URL more descriptive
  • Allows a page slug to be re-used per chapter

Reasons Against

  • Will be less performant as more lookups (Or more advanced queries) to db will be needed.
  • Will complicate code further.
  • Will complicate things when coming to implement another layer of chapter.

@nekromoff
Copy link

Good discussion regarding URLs. From UX perspective, URLs should be as short as possible. Therefore having books/mybook/chapter/new/page/three just adds lots of clutter to URLs.

I think having URL as simple as:
mybook/new/three
makes more sense.

Anyway, this might be another config option to have URL slugs for books, chapters and pages (or not to have them defined at all).

@derek-shnosh
Copy link

derek-shnosh commented Nov 15, 2018

@ssddanbrown, I know this is a bit of an old thread, but I'd like to +1 @nekromoff's suggestion for pretty, or clean URLs.

For example;

Current URL https://docs.domain.local/books/internal-docs/page/client-pkg-virtual-machine-template
Desired URL https://docs.domain.local/internal-docs/client-pkg-virtual-machine-template

As an addition, I think user definable Slugs would be a welcome addition to page options (similar to Page Tags), and maybe even carried over to chapters and shelves, i.e.;

Example uses the same 'Current URL' from above...

Slug URL https://docs.domain.local/internal/vm-client-pkg
Book slug: internal
Page slug: vm-client-pkg

image

@lieszkol
Copy link

lieszkol commented Jan 6, 2020

I'd like to add my vote to implementing slugs. It's difficult to link to pages because a page title can change at any time (thus changing the page URL), and also pages with accented characters end up with very ugly URL-s (an issue also mentioned in issue #1765 ). If one could manually set the page slug, this wouldn't be a problem.
Dan Brown I would be open to help with this if you want.

@ssddanbrown ssddanbrown changed the title Feature Request: Chapter/Page URL Feature Request: Include chapter name with URL path for pages Nov 18, 2020
@rclement
Copy link

@ssddanbrown Sorry to dig up this old issue, but I have encounter a need for URLs with chapter slugs:

  • When ingesting external pages to Bookstack through the API, relative links between pages need are rewritten with deterministic URLs
  • The current /books/<book-slug>/page/<page-slug> URL form is non deterministic because of page slug de-duplication process (if 2 pages in the same book have the same slug, whatever the chapter)
  • Only the /books/<book-slug>/chapter/<chapter-slug>/page/<page-slug> URL form could provide that (given that not 2 pages in the same chapter have the same slug, but that's not Bookstack responsibility)

Would it be possible to have the best of both worlds, such as:

  • Keep the current URL form without chapters for day-to-day operations and best overall performance
  • Allow a page within a chapter to also be referenced with the chapter-based URL (without de-duplicated slugs)?

@ssddanbrown
Copy link
Member

ssddanbrown commented Apr 12, 2022

Hi @rclement,
I understand the need here but I'm hesitant to add another URL scheme we'd need to maintain, especially since it does not directly and fully address the issue you have (Deterministic URLs), but scopes it down to a chapter level.

If you needed this ability right now you could use the below using the logical theme system (Video Guide):

<?php

use BookStack\Facades\Theme;
use BookStack\Theming\ThemeEvents;
use BookStack\Entities\Repos\ChapterRepo;
use BookStack\Exceptions\NotFoundException;
use Illuminate\Support\Facades\Route;

Theme::listen(ThemeEvents::APP_BOOT, function() {
    Route::middleware(['web', 'auth'])->group(function() {
        Route::get('/books/{bookSlug}/chapter/{chapterSlug}/page/{pageSlug}', function(ChapterRepo $chapterRepo, string $bookSlug, string $chapterSlug, string $pageSlug) {
            $chapter = $chapterRepo->getBySlug($bookSlug, $chapterSlug);
            $page = $chapter->pages()->scopes('visible')->where('slug', 'like', $pageSlug . '%')->first();
            if (!$page) {
                throw new NotFoundException(trans('errors.page_not_found'));
            }

            return redirect($page->getUrl());
        });
    });
});

This adds the desired URL endpoint for page-showing, with chapter held within, for example:

https://example.com/books/testing-book/chapter/my-chapter/page/my-page

This will then run a prefix-match using the url page slug, against the pages within that chapter, then redirect to it's standard URL if such a page is found.

Note, since this uses custom internal logic within the logical theme system much of that is technically deemed unstable/not-supported so it's advised to check such customizations upon upgrades.

@rclement
Copy link

Thanks @ssddanbrown for your workaround!

After trying your theme customization snippet, I found an issue with the order of middleware execution:

  • Your original snippet always return a 302 redirection to /login from the browser point of view
  • It only works if web is before auth middleware
  • I am neither a Laravel nor PHP dev but I can relate to any other web framework where you need to unpack cookies/sessions before applying an auth middleware

So here is a working snippet in case anyone needs it:

<?php

use BookStack\Facades\Theme;
use BookStack\Theming\ThemeEvents;
use BookStack\Entities\Repos\ChapterRepo;
use BookStack\Exceptions\NotFoundException;
use Illuminate\Support\Facades\Route;

Theme::listen(ThemeEvents::APP_BOOT, function() {
    Route::middleware(['web', 'auth'])->group(function() {
        Route::get('/books/{bookSlug}/chapter/{chapterSlug}/page/{pageSlug}', function(ChapterRepo $chapterRepo, string $bookSlug, string $chapterSlug, string $pageSlug) {
            $chapter = $chapterRepo->getBySlug($bookSlug, $chapterSlug);
            $page = $chapter->pages()->scopes('visible')->where('slug', 'like', $pageSlug . '%')->first();
            if (!$page) {
                throw new NotFoundException(trans('errors.page_not_found'));
            }

            return redirect($page->getUrl());
        });
    });
});

@ssddanbrown Can you confirm my updated snippet?

@ssddanbrown
Copy link
Member

@rclement Yep! Can confirm your updated snippet. Sorry, was testing on a public dev instance when writing out my version, will update my copy to prevent copying of wrong order.

@martialblog
Copy link

Any updates on this? We also have a lot of repetition in page names. I think having the option to include the chapter in the URL might be beneficial.

@ssddanbrown
Copy link
Member

@martialblog No

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

No branches or pull requests

7 participants