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

Make the named frontend routes customisable #4519

Closed
ErisDS opened this issue Nov 25, 2014 · 10 comments
Closed

Make the named frontend routes customisable #4519

ErisDS opened this issue Nov 25, 2014 · 10 comments
Labels
server / core Issues relating to the server or core of Ghost
Milestone

Comments

@ErisDS
Copy link
Member

ErisDS commented Nov 25, 2014

In core/server/routes/frontend.js there are a number of named routes which use English words including tag, author and page. These need to be abstracted out so that they can be customised for two reasons:

  1. translations: when we support i18n properly, we'll want to allow users to translate those words
  2. flexibility: if you're running a music blog, you might want the route to be /genre/ rather than /tag/, and perhaps you'd rather the route be /by/ rather than /author/.

At the moment, these routes are hardcoded strings repeated multiple times through the codebase, and depended upon for creating logic elsewhere in Ghost.

This needs to be abstracted out into information that can be used to dynamically build the correct routes and assign the right context.

Notes on refactoring

At present there are effectively 3 main contexts which have a top-level route, and a set of subroutes.

  • index - lives at /
  • tag - lives at /tag/:tag_slug/
  • author - lives at /author/:author_slug/

All three of these have the following subroutes:

  • / - the first page of results
  • /page/:page_num/ - subsequent pages of results
  • /rss/ - the first page of the RSS feed
  • /rss/:page_num/ - the subsequent pages of the RSS feed

Also note that at present /feed/ is an alias of the base/index /rss/ route only, when in fact feed should probably be an alias anywhere where rss is used, and should always 301. I can imagine Apps reasonably wanting to specify other aliases for this.

Fundamentally, the main thing we need to achieve is making it possible to replace the words tag, author and page with other words without things breaking, the other information here is intended to paint a bigger picture of what this refactor needs to move us towards.

@ErisDS ErisDS added the server / core Issues relating to the server or core of Ghost label Nov 25, 2014
@ErisDS ErisDS added this to the Next Backlog milestone Nov 25, 2014
@mattiascibien
Copy link
Contributor

Could it be an idea to include them in config.js or should them be made in the DB with a UI like tag management one?

@ErisDS
Copy link
Member Author

ErisDS commented Nov 25, 2014

Eventually we'll move these properties out to the settings or perhaps somewhere else - but for now it doesn't matter - what this issue covers is refactoring the code so that these properties are set in one internal location :)

@mattiascibien
Copy link
Contributor

So I'll give this issue a try. Thanks @ErisDS

@katiefenn
Copy link

@mattiascibien @ErisDS I'm interested in taking a fresh look at this if there has been no recent developments.

@mattiascibien
Copy link
Contributor

@katiefenn did not had time to work on this. :( you can have a look.

katiefenn added a commit to katiefenn/Ghost that referenced this issue Jan 20, 2015
issue TryGhost#4519
- Removal of hard-coded route paths from controller file
- Creation of route config in example config file
- New aliases for other rss paths
- Substitution of hard-coded paths in response context creation
- Added unit tests for new routes and aliases
@katiefenn
Copy link

Are hard-coded URLs in themes within the scope of this refactor?

For example, take tag RSS subscription links in Casper:
https://github.com/TryGhost/Casper/blob/master/tag.hbs#L8
The "tag" keyword is hardcoded, and would break if a tag keyword setting were changed to "genre".

It seems a change to the template is, strictly speaking, outside of the scope of Ghost itself. But should I consider how best to expose a keyword setting to theme templates?

@novaugust
Copy link
Contributor

For themes to be able to route to customized front-end urls, ghost is going to need to add a new {{url-for}} helper that looks up these customized routes.
I'd say that's outside the scope of any PR against this issue, but thanks for bringing it up. We'll have to make a new issue for it, and add all the necessary docs to go along with that :)

@ErisDS
Copy link
Member Author

ErisDS commented Jan 27, 2015

@katiefenn agreed, that's definitely not in the scope of the refactor - also the Casper theme can be updated to use the {{url}} helper to achieve the same thing without hardcoding. I'll update this by way of an example to other themes.

@katiefenn
Copy link

@ErisDS @novaugust Thank you for the feedback. I'm getting ready to create another PR (compare), and wanted to first check that the refactor matches what was asked for:

There are things which I could use some feedback on:

  • Naming of config properties (eg. config.routeKeywords.tag) - could perhaps be simpler.
  • Placing keywords in config. I've concentrated on refactoring the hard-coded strings and put the keywords in config for convenience. If there is a better place, I can move them.

@novaugust
Copy link
Contributor

That compare looks sensible to me. It'll probably be easier to get feedback on the PR if you just open it up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server / core Issues relating to the server or core of Ghost
Projects
None yet
Development

No branches or pull requests

4 participants