piccolo-orm
Add post save, edit, delete hooks
Fixes:
- [x] https://github.com/piccolo-orm/piccolo_admin/issues/64
- [x] https://github.com/piccolo-orm/piccolo_api/issues/162
Thanks a lot for this - it's looking good 👍
The tests are failing, but it looks like something pretty minor.
Sorry for the slow reply. The tests should pass now when updated from master.
Sorry I cannot commit more time to this at the moment. I might come back to it later ...
@ocervell @dantownsend If you both agree, I can make a new PR based on this and fix the linter errors and tests so they can be merged into master (if PR pass review). If not, forget about this comment.
Sorry I cannot commit more time to this at the moment. I might come back to it later ...
@ocervell No problem - I think it's mostly done, just some final tweaks 👍
@sinisaos Yes, that would be helpful, thanks. I think the only outstanding question (I think it was related to this PR, but maybe not) was I think the POST endpoint returns a list containing the ID instead of just the ID. To change it would be a breaking change, but might make more sense.
@sinisaos Yes, that would be helpful, thanks.
OK. I will make a new PR based on this PR with linter errors, some typos in the docs and tests fixed.
I think the only outstanding question (I think it was related to this PR, but maybe not) was I think the POST endpoint returns a list containing the ID instead of just the ID. To change it would be a breaking change, but might make more sense.
No, it's not in this PR or any PR. This is mention in issue #211.
UPDATE:
I just checked and nothing actually breaks (I don't know if you have more complicated projects that could be affected by this change). Only one FastAPI test failed ([{"id": 2}] should be {"id": 2}). Piccolo Admin also works without changes. Hope that helps.
@JoshYuJump There was a #231 based on this PR with fixed linter errors and tests., but I closed it due to lack of interest in it. Feel free to use the code from that PR and redeploy it, if there is interest in implementing it now.
@JoshYuJump There was a #231 based on this PR with fixed linter errors and tests., but I closed it due to lack of interest in it. Feel free to use the code from that PR and redeploy it, if there is interest in implementing it now.
@sinisaos Thanks your solution, I used the code from the PR, it works fine.
@JoshYuJump Thanks. Yes, PR works and passes all tests and linter checks and is ready to merge. Feel free to use the code from that PR, you might have better luck and your PR will be merged.
P.S. There are a few more PRs (#145, #160, #226) with interesting features that I closed because they were open for too long and I gave up on them. Check them out if you want and feel free to use the code for your PRs (if you're interested in making PRs) as you might have better luck getting them merged. I think they are good and useful and will make the library better.