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

Feature Request: AfterEager, AfterAll, BeforeFind, BeforeAll callbacks #557

Open
Jwonsever opened this issue May 31, 2020 · 2 comments
Open
Labels
f: associations the associations feature in pop proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue

Comments

@Jwonsever
Copy link

Related: #476

Pop callbacks are limited to afterFind, and before/afterEdit* type hooks.

The Afterfind hook is nice, however it doesn't have any eager relationships, and it can cause performance issues when all is called. Also, there is no way to, in pop, make adjustments or modifcations before a find call (such as to enforce an access check). These issues mean it ends up only being useful for a very limited set of tasks where the relationships are not necessary and there are no DB calls. (This is further exacerbated by #530, however that is a separate issue.)

This means code using Pop needs to write its own implementations of these methods; instead of calling tx.All(model), we have to call model.All(tx). Ideally, the interface provided by pop would be more extensive, and users would be able to use the connection and query functions for all use cases.

Requested Behaviors

  • AfterEager
    AfterEager would be called after all Eager relationships are loaded in a find.
  • AfterAll
    AfterAll would be called after an All request.
  • AfterAllEager
    AfterEagerAll would be called after an All request after all realtionships are loaded.
  • BeforeFind
    BeforeFind would be called directly before loading a record.
  • BeforeAll
    BeforeAll would be called directly before an all call.

Additionally, there should be a way to have afterfind/ aftereager not get called when making an all request. This would be a breaking change if we take that away, so maybe we do something similar to eager modes where we can change the callback behavior?

I'm aiming to start a conversation here, so I hope we can get some feedback on this proposal and then move towards some PR's and get these capabilities in.

Info

I am primarily using Pop through Buffalo on macOS.

@ayuayue
Copy link

ayuayue commented Jun 4, 2020

I need ,too

@stanislas-m stanislas-m added the proposal A suggestion for a change, feature, enhancement, etc label Jun 20, 2020
@Jwonsever
Copy link
Author

@stanislas-m Thinking about putting together a PR for these. Anything to watch out for?

@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Sep 20, 2022
@sio4 sio4 added this to the v6.1.0 milestone Sep 20, 2022
@sio4 sio4 added the f: associations the associations feature in pop label Sep 20, 2022
aeneasr added a commit that referenced this issue Jan 2, 2023
See #557
Closes #476

Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: associations the associations feature in pop proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue
Projects
None yet
Development

No branches or pull requests

4 participants