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

API: No more staticPages parameter + better pages support #5151

Closed
ErisDS opened this issue Apr 16, 2015 · 3 comments
Closed

API: No more staticPages parameter + better pages support #5151

ErisDS opened this issue Apr 16, 2015 · 3 comments
Assignees
Labels
affects:api Affects the Ghost API

Comments

@ErisDS
Copy link
Member

ErisDS commented Apr 16, 2015

Note: this issue is pretty blocking for the {{#get}}

At the moment, the Posts Browse endpoint supports a property called staticPages. If set to true, this includes any posts which have page: true set on them (pages). Perfect for the admin UI! However, it's not great as a general API feature. There is currently no way to fetch just pages... which is weird, and the staticPages parameter is horribly named.

The ideal thing here, I think, is to pass do api.post.browse({page: true}) as that matches the name of the property in the database. We should also support api.post.browse({page: null/all}) (so that the possible values are true, false or either null or all, depending on what makes sense) so that we can still support returning both for the time being.

In addition, it would be best to create an alias of page for posts with page=true, at least in the helper itself, so that you can do {{#get 'pages'}} instead of {{#get 'posts' page="true"}}.

The blocker for doing this is #5093, because page in the API currently refers to pagination, rather than the page property. If we use offset directly, instead of page for pagination in the API, not only do we get a more flexible interface, but we also get the page keyword back for use for actual pages!

Two other possibilities would be

  1. to have api.post.browse({isPage: true}) in which case we'd probably want consistency in api.post.browse({isFeatured: true}) - not sure about this.
  2. to have api.post.browse({postOrPage: page}) - I'm very wary of anything generic like type because that just spreads the idea there might be more options like this in future.

In the long run, pages and featured might get reimplemented as hidden/private tags instead of properties on the post model itself, so I think keeping the most simple interface (page:true) is best?

@ErisDS ErisDS added the affects:api Affects the Ghost API label Apr 16, 2015
edsadr added a commit to edsadr/Ghost that referenced this issue Apr 22, 2015
    Closes TryGhost#5152, TryGhost#5093, TryGhost#5151
    - Adds `featured` filter option to posts.browse method modifying the model to take it too
    - Implements offset pagination method to posts.browse method modifying the model to work with it too but still supporting the `page` parameter
    - Removes `staticPages` parameter in options filter to allow using the `page` parameter with options `all` `true` or `false
@ErisDS ErisDS mentioned this issue Jun 30, 2015
31 tasks
@ErisDS ErisDS added this to the Current Backlog milestone Jun 30, 2015
@ErisDS ErisDS modified the milestone: Current Backlog Oct 9, 2015
@ErisDS ErisDS self-assigned this Oct 13, 2015
@ErisDS ErisDS modified the milestone: Public API v1 Oct 13, 2015
@ErisDS
Copy link
Member Author

ErisDS commented Oct 18, 2015

I think we need to start adding treatment of pages as a first-class citizen in the API:

api.pages.browse({slug: 'about'}) and {{#get "pages" slug="about"}}

With this change, there is an opportunity to completely remove the ability to retrieve pages via the posts endpoint (and get rid of the staticPages parameter that makes me cringe every time I see it 😖!). This would mean it is no longer possible to return both posts and pages from the API at the same time.

I believe this is the right route to go down, the difference between what posts and pages 'are' conceptually means that is is really uncommon to want to deal with both at the same time. The driving use case for the current implementation is the current admin content screen, where with the recent redesign there it is no longer necessary to have them together (and perhaps not even ideal).

Making this split once and for all (and before we start making access to the API the norm) means that we can more easily hide away the implementation of pages, so if we wanted to separate them into their own table, or implement them with a special tag rather than with a flag on the post table, we could.

cc @JohnONolan && @kevinansfield if I made this change, we could either recreate the content list in the admin UI by doing two requests, or we could add an extra item in the lefthand navigation so users can switch between posts & pages. I'd love to get thoughts on which is best (or if there's another solution) and to get the go-ahead to make this change.

@kevinansfield
Copy link
Member

I agree with the split in the api endpoints as I think outside of the current admin content screen you'll always be filtering by one or the other. The only time I can think where you want to retrieve both would be search but then that would have it's own endpoint.

Personally I'd lean towards adding a separate Pages item to the nav menu as it would make a clear distinction between posts/pages and how they are treated differently on the front-end. What were the arguments against a split previously or were the options constrained more by the previous design than anything else?

@JohnONolan
Copy link
Member

What were the arguments against a split previously or were the options constrained more by the previous design than anything else?

Essentially that pages are rarely used, so it's questionable how much UI real estate they deserve to redundantly take up. Typically an about/contact/whatever page is set up, and then nothing changes for long, long time.

@kevinansfield
Copy link
Member

Copying relevant discussions from Slack...

Main issues of splitting posts/pages into separate resources from the ember side:

  • toggling the model type could be tricky, not sure how to handle that with ember data yet
  • (future issue?) tag objects returning relations will need to specify both post_ids and page_ids
  • if we’re keeping the same UI without splitting pages and posts into their own areas then pagination of the content list could become rather difficult

Followup:
I think toggling a model between page/post type should be doable by creating a new model of the appropriate type using the old models attributes and pushing it into the store then removing the previous one. The only place to toggle between post/page at the moment is in the PSM on the editor screen so as long as all the templates stay the same it would be possible to switch everything out underneath whilst re-using the same DOM elements.

That still leaves the singular content list and dealing with pagination unless we split them out in the UI or have an endpoint that returns both.

sebgie added a commit to sebgie/Ghost that referenced this issue Nov 4, 2015
refs TryGhost#5151
- disable staticPages parameter for calls without authentication
@ErisDS ErisDS modified the milestone: Public API v1 Dec 3, 2015
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Sep 20, 2016
@ErisDS
Copy link
Member Author

ErisDS commented Sep 20, 2016

I'm closing most API issues temporarily with the later label.

JSON API Overhaul & OAuth access are currently scheduled next on the roadmap

@ErisDS ErisDS closed this as completed Sep 20, 2016
@ErisDS ErisDS removed later [triage] Things we intend to work but are not immediate priority labels Jan 23, 2019
naz added a commit to naz/Ghost that referenced this issue Sep 17, 2019
naz added a commit that referenced this issue Sep 17, 2019
refs #5151
refs #10737

- Removed all uses/references to post's "staticPages" filter
- It was only a feature specific to API v0.1 which doesn't have to take space in the codebase anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:api Affects the Ghost API
Projects
None yet
Development

No branches or pull requests

3 participants