search

ponzu-cms / ponzu

Headless CMS with automatic JSON API. Featuring auto-HTTPS from Let's Encrypt, HTTP/2 Server Push, and flexible server framework written in Go.
https://docs.ponzu-cms.org
BSD 3-Clause "New" or "Revised" License
5.7k stars 383 forks source link

Adds BeforeAPIResponse and AfterAPIResponse hooks #305

Closed olliephillips closed 5 years ago

olliephillips commented 5 years ago

As discussed in issue #302, this provides two new hooks so that we can interact with the JSON response from the Ponzu API. Use cases mentioned so far include spellchecks, shortcode replacement and sanitization.

The function signatures are slightly different to the other hooks as the []byte slice representing the JSON response is passed to both new hooks, and in the case of BeforeAPIResponse also returned ready to be sent by Ponzu.

BeforeAPIResponse(http.ResponseWriter, *http.Request, []byte) ([]byte, error)
AfterAPIResponse(http.ResponseWriter, *http.Request, []byte) error

After APIResponse is worth having in my mind, it may be useful for logging or notifications.

If this is OK, I will of course also amend the docs to reflect the new hooks, and make a separate pull request.

No rush on feedback, appreciate you are busy, just wanted to send now as I'm liable to get distracted and forget about it myself :)

olliephillips commented 5 years ago

Hey @nilslice. I've been playing with the shortcode plugin some more. That obviously can't work until this PR is merged, but it leads me to think about the addons API and whether it needs extending. Link to the license? Link to a buy license if applicable? And certainly a link to documentation for the add-on? Just thoughts.

I was also interested in your marketplace ideas - I think we had a short discussion about this some time back.

I note you haven't implemented a payable model on FBScheduler yet.

I appreciate, it's a bit rude to ask, but is this down to lack of interest in the addon? Or did you think it vulgar to offer paid pieces on top of an open source system? I don't subscribe to that myself, but I've had a few other ideas for add-ons, which would be pointless in pursuing, if there's no commercial element to Ponzu add-ons.

olliephillips commented 5 years ago

This PR is now a month old and there's been no feedback on the request nor the subsequent points on the addon API. I expect you're busy enough @nilslice, so no problem, but do the @ponzu-cms/team have any thoughts on the submitted code or API discussion.

nilslice commented 5 years ago

@olliephillips, Sorry! Yes, quite busy at the moment.. Will answer the last couple here:

This PR is now a month old and there's been no feedback on the request nor the subsequent points on the addon API. I expect you're busy enough @nilslice, so no problem, but do the @ponzu-cms/team have any thoughts on the submitted code or API discussion.

I'm fine with pulling the additional methods in and going with your implementation if it works for you. Maybe give the rest of @ponzu-cms/team a day or so to respond with thoughts. Otherwise, take this on as an exercise in your first merge to the project as a member :) The docs should be updated as well, which live in 2 places (😞, I know...) -- I can get to them eventually, unless you want to poke around https://github.com/ponzu-cms/docs and modify/submit.

I've been playing with the shortcode plugin some more. That obviously can't work until this PR is merged, but it leads me to think about the addons API and whether it needs extending. Link to the license? Link to a buy license if applicable? And certainly a link to documentation for the add-on? Just thoughts. I was also interested in your marketplace ideas - I think we had a short discussion about this some time back. I note you haven't implemented a payable model on FBScheduler yet. I appreciate, it's a bit rude to ask, but is this down to lack of interest in the addon? Or did you think it vulgar to offer paid pieces on top of an open source system? I don't subscribe to that myself, but I've had a few other ideas for add-ons, which would be pointless in pursuing, if there's no commercial element to Ponzu add-ons.

I had high hopes of doing something like a marketplace for addons, but never got around to solidifying the API for addons. I agree the API may need extending, so please feel free to add as you see fit.

With regards to the FBScheduler, I should update the repo to remove that notice about the license. I think the whole concept of a paid/free addon marketplace would be a huge asset to the project, but it will take some more thought and work on the APIs. All that is to say, make whatever modifications you'd like to see and we can regroup on this.

olliephillips commented 5 years ago

Now includes repo /docs amends to cover the new hooks

olliephillips commented 5 years ago

Hey @nilslice, I have to confess I'm not sure where we are up to on this now a bit of time has elapsed. Are you merging to master or team members? I can see you're set at busy so no urgency, just wanted to check in. If team is that best effected by cloning the repo and merging the branches. I can't see anything on github to do it.

nilslice commented 5 years ago

@olliephillips -- sorry for delay... this project has admittedly escaped my attention and wish I had more time to dedicate to it at the moment. I forgot I had merged this into ponzu-dev, but if it is ready, feel free to merge to master. I believe you'll need to pull locally, and then create a PR to merge ponzu-dev to master. You should be able to merge to master as a team "maintainer" (which is your current status set in repo settings).

Let me know if this is not the case and I can take a closer look -- as far as I can tell, the settings will allow you to do it though.

olliephillips commented 5 years ago

You didn't forget. Initially team members could take this as far as merge to ponzu-dev, but you needed to merge to master. At least that's where we started. I think you might have opened that up.

I'll sort the merge - is there anything special to do for a release, is it done via the CI or do we tag it?

nilslice commented 5 years ago

I'll sort the merge - is there anything special to do for a release, is it done via the CI or do we tag it?

Thank you! There are some CI tests run, which do a simple integration test... nothing major. They don't impact any release though. Releases have been done manually by creating a release & tag here (https://github.com/ponzu-cms/ponzu/releases/new) - no pre-built binaries are added historically.. mainly since the Go toolchain is a requirement to use Ponzu anyways.

olliephillips commented 5 years ago

No, can't push on master. Seems to need a review from user with write access permissions.

ponzu-dev branch seems to have accepted the version increment though.

nilslice commented 5 years ago

Right, you don't want to push to master, you want to create a PR from the ponzu-dev branch whose base is master.

https://github.com/ponzu-cms/ponzu/compare/ponzu-dev?expand=1

That URL can be reached by clicking the "Branches" tab on the main page of the repo, and then "New Pull Request".

olliephillips commented 5 years ago

Ok done that. It seems it still needs approving, but I guess you and others can see this on Github now.