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

Refactor to using pipeline for the API #5467

Merged
merged 1 commit into from
Jun 30, 2015
Merged

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Jun 22, 2015

This is an example as a talking point, it's not intended to be merged

refs #2758

  • Shows how an API method could be refactored to use pipeline
  • Each functional code block is a named task function
  • Each function takes options, manipulates it, and returns options back
  • Tasks like permissions can reject if they don't pass, causing the pipeline to fail
  • Tasks like validating and converting options might be abstracted out into utils - the same for each endpoint
  • Tasks like the data call can be extremely complex if needs be (like for some user endpoints)

@ErisDS ErisDS force-pushed the api-pipeline branch 7 times, most recently from 93244a3 to 923f5f7 Compare June 25, 2015 12:35
@ErisDS
Copy link
Member Author

ErisDS commented Jun 25, 2015

I've updated this to cover all of posts, users and tags. I think the code is far more readable - and it's at least a lot clearer where the complexities are in the user API. CodeClimate hates me though - too much similar code!

The second commit refactors duplication out of the validation.

There are still like a gazillion refactors I could do here, but I'm thinking to add unit test coverage for api/utils.js, clean up the docs, squash and call this done. Any thoughts?

Update/P.S. - I plan to open up a beginner issue to roll out the pipeline to other endpoints.

@ErisDS ErisDS changed the title [Example] using pipeline for the API Refactor to using pipeline for the API Jun 25, 2015
@ErisDS
Copy link
Member Author

ErisDS commented Jun 25, 2015

Updated to include test coverage for api utils and tweaked the inline docs a bit (could be better). It should pass the build & I think it's good to go.

From here the plan is that I can sort out API option handling more easily, and also start to put in place a way to determine permissions for endpoints which are public (but have requirements like post status being published) so that we might be able to ship proper access to those endpoints using client auth.

@ErisDS
Copy link
Member Author

ErisDS commented Jun 27, 2015

This will pass once #5488 is merged and this is rebased on it

refs TryGhost#2758

- Post, Tag & User API methods are refactored to use pipeline
- Each functional code block is a named task function
- Each function takes options, manipulates it, and returns options back
- Tasks like permissions can reject if they don't pass, causing the pipeline to fail
- Tasks like validating and converting options might be abstracted out into utils - the same for each endpoint
- Tasks like the data call can be extremely complex if needs be (like for some user endpoints)
- Option validation is mostly factored out to utils
- Option conversion is factored out to utils
- API utils have 100% test coverage
- Minor updates to inline docs, more to do here
sebgie added a commit that referenced this pull request Jun 30, 2015
Refactor to using pipeline for the API
@sebgie sebgie merged commit a0d0045 into TryGhost:master Jun 30, 2015
@ErisDS ErisDS deleted the api-pipeline branch September 27, 2015 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants