apostrophecms / apostrophe

A full-featured, open-source content management framework built with Node.js that empowers organizations by combining in-context editing and headless architecture in a full-stack JS environment.
https://apostrophecms.com
Other
4.36k stars 590 forks source link

_url property for piece types at the homepage returns "baseUrl//slug" #3650

Closed martibravo closed 2 years ago

martibravo commented 2 years ago

To Reproduce

Step by step instructions to reproduce the behavior:

  1. Set a piece-page-type index as your root URL. (the index.html should show up when accessing the baseUrl, in this case http://localhost:3000
  2. Create a piece of the type.
  3. In templates, piece._url returns "localhost:3000//:slug" instead of "localhost:3000/:slug"

Expected behavior

piece._url should not have a double slash.

Describe the bug

There is an extra slash in piece-page-types show page when the index is set as the root of an Apostrophe website.

Details

Version of Node.js: v16.13.2

Server Operating System: Ubuntu inside Windows Subsystem for Linux

martibravo commented 2 years ago

Proposed solution:

// /modules/@apostrophecms/piece-page-type/index.js

methods(self) {
    return {
      buildUrl(req, page, piece) {
        if (!page) {
          return false;
        }
        return page.slug === '/' ? page._url + piece.slug : page._url + '/' + piece.slug;
      },
     //other methods
}
boutell commented 2 years ago

Good catch. Using apos.util.addSlashIfNeeded is the simplest fix here. Would you like to submit a PR?

On Mon, Feb 7, 2022 at 5:04 AM Martí Bravo @.***> wrote:

Proposed solution:

// @.***/piece-page-type methods(self) { return { buildUrl(req, page, piece) { if (!page) { return false; } if (page._url === self.apos.options.baseUrl) { return page._url + piece.slug; } return page._url +'/'+ piece.slug; }, //other methods}

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3650#issuecomment-1031283285, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OJ2GQZ3TNQDNPODF3UZ6KLHANCNFSM5NW6RVVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

martibravo commented 2 years ago

@boutell Already done :)