Allow hooks into responds that touch Ecto to provide conn and model or errors

[joshsmith]

In my app prior to using ja_resource, I often hooked into the success of a Repo.insert or update in order to track analytics on the action that just happened for a model, like so:

{:ok, my_model} ->
  conn
  |> @analytics.track(:added, my_model)
  |> put_status(:created)
  |> render("show.json-api", data: my_model)

It would be very useful for me to be able to have this behavior going forward.

Given that you have a defoverridable in JaResource.Create, Update, etc for handle_ actions, was thinking it could be possible to provide a similar override for your created, updated, etc, and invalid responses.

This would allow me to pass along a conn and model or a conn and errors so that I can hook into these behaviors to do my analytics tracking and any other responding that needs to happen before allowing the response to continue as normal.

Given how integral to my application tracking analytics is, this is definitely a high priority for me.

This is just my first suggestion when thinking of a way to tackle this, so would be happy to hear a different approach if you think it's unfeasible for some reason.

[alanpeabody]

This is an interesting idea. Ideally I would like as much flexibility as possible.

I was originally thinking about an "after_insert" type callback, but I like the ability to override the rendering as well.

I also think the repo insertion should be customizable.

Do you think the three callbacks would be enough? In particular would a shared invalid callback be an issue?

[joshsmith]

@alanpeabody yeah, I think being able to override rendering is a nice side effect of this instead of doing an after_insert like callback. Also seems a bit more natural to how I currently write my APIs (prior to moving to ja_resource, that is).

I think we'd at least want callbacks like:

• handle_created
• handle_updated
• handle_deleted
• handle_invalid

While I don't need _invalid right now, I can see the utility in it.

What do you think about that?

In particular would a shared invalid callback be an issue?

Can you expand on what you mean by this?

[alanpeabody]

@JoshSmith I mean with both create and update sharing the same invalid hook.

Otherwise what you describe above makes sense to me and I think it fits the lib well.

[joshsmith]

Ah I got you! I'd be fine with it personally, although am also very happy not even having the invalid portion handled yet given personal priorities right now.

Let me know if you'd like to brainstorm the exact approach here and maybe we can help with a PR on it. Would definitely like to be sure I understand how you'd like this implemented.

[alanpeabody]

I think to simplify implementation handle_invalid_update and handle_invalid_create might be better.

Implementation wise I think they should be callbacks with default implementations that call the existing created/invalid functions now made public. The controller can get passed into respond and then it is just something like:

def respond(%Plug.Conn{} = conn, _conn, _controller), do: conn
def respond({:ok, struct}, conn, controller), do: controller.handle_updated(conn, struct)
def respond({:error, errors}, conn, controller), do: controller.handle_invalid_update(conn, errors)
def respond(other, conn, controller), do: controller.handle_invalid(conn, other)

[alanpeabody]

Actually, is handle_update vs handle_updated clear enough? Maybe render_update and handle_update instead?

[joshsmith]

@alanpeabody I would be fine with either. Are you expecting like in handle_update to just pass the values back again, i.e. just let it chain?

[alanpeabody]

@JoshSmith handleupdate doesn't need any changes, but all of the render callbacks should return a conn. Is that what you are asking?

[joshsmith]

@alanpeabody yeah, that's what I had meant.

[psteininger]

Closing this, as it appears to be merged