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

[Epic] Frontend Refactor (Channels) #5091

Closed
7 of 14 tasks
ErisDS opened this issue Mar 31, 2015 · 6 comments
Closed
7 of 14 tasks

[Epic] Frontend Refactor (Channels) #5091

ErisDS opened this issue Mar 31, 2015 · 6 comments
Assignees
Labels
server / core Issues relating to the server or core of Ghost

Comments

@ErisDS
Copy link
Member

ErisDS commented Mar 31, 2015

This is a WIP epic - I'm still working out the steps needed to get from A-B and I'll break it down into sub tasks/sub issues as and when, in the meantime, the goal is clear and documented here.


The frontend of Ghost - that is the server side code which renders a blog using the active theme - has been somewhat unloved for a little while now. It essentially consists of some hard coded routes in /routes/frontend.js, which in turn call the monolithic functions in /controllers/frontend.js. It's time for this part of Ghost to be refactored into something more elegant, and importantly, something more extensible.

On the wiki you'll find a new document, titled Channels 101, which introduces and explains the concept of Channels for the frontend. In short channels are the abstracted concept of a list of posts, i.e. the homepage, the tag archive, the author archive or any other group of posts you could conceive of.

And the very end of the document it talks about working to slowly remove hardcoded bits of the fontend and refactor it into configuration & generation code, before finally moving towards making channels a model & API endpoint just like Posts, Tags and Users.

This issue is the 'epic' where we'll keep track of our progress towards achieving this New World Order.

Need to solve:

  • Dynamic routing for channels
  • Pagination when in channel
  • meta data for channels
  • sitemap for channels
  • Improve the channels API

Maybe later:

  • add a channel registration mechanism that works whilst Ghost is running
  • Support offset, rather than page, in the API Paginate all the things #2896
  • RSS feeds for each channel linked in that channel
  • Post urls & next/prev post when in channel

Updates: Aug 2017

The current state of channels is that it the functionality is largely refactored and in place, but can only be accessed by modifying the core file channels-config.js. This is because, although the basics are working, advanced behaviour of Ghost isn't yet properly linked to channels.

Examples of missing pieces (some listed above) are:

  • Sitemaps
  • Meta data
  • RSS feed (URLs only)
  • Next/prev post when in a channel
  • Deep customisation, e.g. disabling RSS feeds

There is also some pure-refactoring that needs to be done:
E.g.

  • moving channels to a proper home
  • moving non-channel pieces from controllers/frontend.js to a proper home
  • strengthening the implementation to make channels more of a first class object
  • better/clearer default configuration
  • proper use of express
  • exposing channels without core modification

Basically, we need to iron out a few more of the major pieces, and then expose this config. Short term, we're going to start publishing blog posts on how to use the functionality that is there by hacking core.

@ErisDS ErisDS added epic server / core Issues relating to the server or core of Ghost labels Mar 31, 2015
@ErisDS ErisDS added this to the Current Backlog milestone Mar 31, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Apr 10, 2015
refs TryGhost#5091, refs TryGhost#2263

- Move rss handling out of the frontend controller and into its own module
- Separate the code into logical blocks
- Wrap the generation code in a in-memory cache to prevent it being regenerated on every request
ErisDS added a commit to ErisDS/Ghost that referenced this issue Apr 10, 2015
refs TryGhost#5091, refs TryGhost#2263

- Move rss handling out of the frontend controller and into its own module
- Separate the code into logical blocks
- Wrap the generation code in a in-memory cache to prevent it being regenerated on every request
ErisDS added a commit to ErisDS/Ghost that referenced this issue Apr 10, 2015
refs TryGhost#5091, refs TryGhost#2263

- Move rss handling out of the frontend controller and into its own module
- Separate the code into logical blocks
- Wrap the generation code in a in-memory cache to prevent it being regenerated on every request
@ErisDS ErisDS mentioned this issue Apr 12, 2015
@ErisDS ErisDS changed the title [WIP] [Epic] Frontend Refactor (Channels) [Epic] Frontend Refactor (Channels) May 26, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue May 26, 2015
refs TryGhost#5091

- This is step one of several steps towards ending up with dynamic routes for channels
- Refactoring this way makes the similarities between all the routes clearer to see
ErisDS added a commit to ErisDS/Ghost that referenced this issue May 27, 2015
refs TryGhost#5091

- This is step one of several steps towards ending up with dynamic routes for channels
- Refactoring this way makes the similarities between all the routes clearer to see
ErisDS added a commit to ErisDS/Ghost that referenced this issue May 30, 2015
refs TryGhost#5091

- adds names to all middleware functions, for debugging purposes
@ErisDS ErisDS modified the milestones: Current Backlog, Zelda Jun 2, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Jun 2, 2015
refs TryGhost#5091

- adds names to all middleware functions, for debugging purposes
ErisDS added a commit to ErisDS/Ghost that referenced this issue Jun 2, 2015
refs TryGhost#5091

- adds names to all middleware functions, for debugging purposes
@ErisDS ErisDS modified the milestones: Current Backlog, Zelda Jun 2, 2015
ErisDS added a commit to ErisDS/Ghost that referenced this issue Oct 20, 2017
refs TryGhost#5091

- There is very little that changes here, just code readability
- However I've expanded out the tests getting ready to be able to test more deeply as I refactor the routing
ErisDS added a commit that referenced this issue Oct 23, 2017
refs #5091

- There is very little that changes here, just code readability
- However I've expanded out the tests getting ready to be able to test more deeply as I refactor the routing
ErisDS added a commit that referenced this issue Oct 24, 2017
refs #5091

- remove the use of functions
- remove unnecessary quotes from tag filter
- move channel config to be a JSOn file called config.channels.json
- accept external config
- new channelUtils for tests
- remove channelConfig.get 
- refactor so tests work as expected
- refactor away duplicate 'name' value
ErisDS added a commit that referenced this issue Oct 25, 2017
refs #5091

- Move all of the code to do with handling channels into one folder
- Still keeping all the shared/simlar code for rendering etc inside weird
  frontend folder until I am sure what this will look like
ErisDS added a commit that referenced this issue Oct 26, 2017
refs #5091

- rkw is something I made up on the spot
- t_ looks like a translation function, which is what this IS!
ErisDS added a commit that referenced this issue Nov 5, 2017
refs #9192, refs #5091

- Using a class allows for easy shared logic
- Loading is designed to work from config right now, but could be DB driven, etc
- Provided configuration can be simplified and extended in the constructor / class methods
- Update tests, move custom assertions to utils
ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 6, 2017
ref TryGhost#5091

- render channel was always a weird file
- now it's clearly 2 things
- we're slowly getting towards closing 5091... 🎉
ErisDS added a commit that referenced this issue Nov 6, 2017
refs #5091, refs #9192

- render channel was always a weird file
- now it's clearly 2 things
- we're slowly getting towards closing #5091... 🎉
- added some extra tests
ErisDS added a commit that referenced this issue Nov 7, 2017
refs #5091, refs #9192

- This is similar to #9218, in that I'm revealing bits of code that are "controllers" in our codebase. As opposed to routes, services, renderers etc.
- This also reveals some code which is identical to the channels controller
- There is more to do here, but for now I've got the module split up, and the tests split and improved.
- Next I'll split RSS into controller + service, DRY up the controller code, etc
ErisDS added a commit that referenced this issue Nov 8, 2017
refs #5091, #9192, #9178

- Get the RSS module into a much better shape
- Controller -> /controllers/rss
- Remainder -> /services/rss
- Moved tests to match & updated requires
ErisDS added a commit that referenced this issue Nov 8, 2017
refs #5091, refs #9192

- There are several theme template "renderers" all over the codebase
- Some are in apps, and were called "controllers"
- One is in error handling
- All of them now have comments marking out how they share logic/steps
- Other comments describe routes & controllers where they live
ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 8, 2017
refs TryGhost#5091, refs TryGhost#9192

- There are several theme template "renderers" all over the codebase
- Some are in apps, and were called "controllers"
- One is in error handling
- All of them now have comments marking out how they share logic/steps!
ErisDS added a commit that referenced this issue Nov 8, 2017
refs #9192, refs #5091, refs #9178

- moved channels from controllers to a service
- split out the parent router from the remaining individual router logic
- moved the tests to match
ErisDS added a commit that referenced this issue Nov 9, 2017
refs #9192, #5091

- changed channels to use our new base class
- keep the flexible structure, so that channels can be reloaded
- I had to move the router into the route service otherwise we get circular dependencies
- Don't _really_ want to keep it like this - need a way to define base classes as shared
ErisDS added a commit that referenced this issue Nov 10, 2017
refs #9192, refs #5091 

- Moved all url generation into generate-feed.js, so we can see as much data processing as possible in a single place.
- Refactored the way res.locals were used, to be more like how express uses them prior to rendering
- Removed a bunch of code & tests todo with context for RSS - I can't see any way that'd be used, unless we switched the rendering to use a template.
- moved the RSS rendering to be part of the service, not controller
- updated the tests significantly 

Note: RSS generate-feed has a complete duplication of the code used in the excerpt helper in order to create an item description
ErisDS added a commit to ErisDS/Ghost that referenced this issue Nov 10, 2017
refs TryGhost#5091, TryGhost#9192

- Renderer figures out templates, contexts, and does a render call
- Renderer will handle everything after data is fetched and semi-processed
- For now it only does the last 2 steps
- TODO: it will eventually do more steps!!
ErisDS added a commit that referenced this issue Nov 10, 2017
refs #5091, #9192

- Renderer figures out templates, contexts, and does a render call
- Templating is now handled with a single function
- Context call is made in the renderer

Note:  to make this work, all controllers now define a little bit of config, currently stored in res._route. (That's a totally temporary location, as is res._template... when a sensible naming convention reveals itself I'll get rid of the weird _). This exposes a type and for custom routes a template name & default.
@RaringCoder
Copy link

How far off completion is this? I'd love to contribute but I'm no Node developer lol

@ErisDS
Copy link
Member Author

ErisDS commented Apr 24, 2018

Closing in favour of #9601

@ErisDS ErisDS closed this as completed Apr 24, 2018
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