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

Adding links/metadata in dashboard #1197

Open
grncdr opened this issue Dec 20, 2023 · 2 comments
Open

Adding links/metadata in dashboard #1197

grncdr opened this issue Dec 20, 2023 · 2 comments

Comments

@grncdr
Copy link
Contributor

grncdr commented Dec 20, 2023

Is there a supported way to extend GoodJob views with additional metadata?

The use case for me is linking to admin backends and observability tools. For example, we already have a centralized log collector and a log viewer that can handle searching by Job ID. I'd love to be able to add a link on the good_job/jobs#show view that takes me straight to those logs.

Similarly, we have an admin backend with a search that supports searching by GlobalID (e.g. "gid://theapp/User/123") since job parameters are often serialized as Global IDs, it could be trivial to show an admin link from the job details.

Idea

I know I can just override app/views/good_job/jobs/show.html.erb within my own app, but then I'm on the hook for pulling in upstream changes whenever we upgrade GoodJob. At the same time, I don't want to introduce a heavyweight solution. What do you think about adding an empty partial that is rendered within that view, that users can explicitly override within their applications?

In practice that would look like:

Update show.html.erb

+ <%= render 'custom_details' %>
  <div class="my-4">
    <h5>
      <%= t "good_job.models.job.arguments" %>

Add _custom_details.html.erb:

<%# This partial is intentionally blank. You can override it by adding app/views/good_job/jobs/_custom_details.html.erb in your app. %>

With that change, users can add what they need in a way that should impose relatively little maintenance burden for anyone.

@bensheldon
Copy link
Owner

Oh I like this! I also like that it's trivial to override the custom template from the main app.

For that custom template, I imagine that it should pass in the GoodJob::Job parameter. That sort of exposes a model that I don't consider public/stable, so I'm not sure what would be the recommendation for throwing a test around it, but I like the overall idea.

@grncdr
Copy link
Contributor Author

grncdr commented Dec 21, 2023

For that custom template, I imagine that it should pass in the GoodJob::Job parameter. That sort of exposes a model that I don't consider public/stable, so I'm not sure what would be the recommendation for throwing a test around it, but I like the overall idea.

My initial thoughts on that:

  1. Partials have access to instance variables, so a user can always poke around in @job, stable interface or not. The same applies to view helpers. Personally, I think simply documenting "this is experimental, no stable API" is sufficient to start.
  2. For my examples, I mostly use @job.active_job anyways, so a view helper to return that (strawman: current_active_job) would provide most of the necessary API without exposing GoodJob models.
  3. However, I also want to extend the view of individual executions, which puts me right back to depending on a non-public model.

Subjectively, I think exposing an internal/unstable model is the right trade-off in this context. Anything else requires an indirection layer that will need to be implemented and maintained, and that users will need to learn. The risk that somebody updates good job and cuts a release without checking if the views still work, and that this is critical functionality, seems pretty low to me.

I've got some further ideas to improve the DX, but I'll try to open up a PR soon where those can be discussed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Inbox
Development

No branches or pull requests

2 participants