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

Pass associated_class to collection from show #2238

Conversation

stormmaster42
Copy link
Contributor

This fixes an issue where the embedded collection is using the parent's class to define i18n headers.

Example: Ressource order has_many order_items.
Order dashboard declares order_items: Field::HasMany

When the show which includes the order_items association is rendered within the collection partial it uses the parent's resource (Order) instead of OrderItem to determine the header label default.

Not sure how to add a test for this behavior.

this fixes an issue where the embedded collection is using the parent's class to define i18n headers
@pablobm
Copy link
Collaborator

pablobm commented Aug 11, 2022

Thank you for the MR 🙂 I'm trying to reproduce the issue locally, but I'm not having any luck. Would you be able to explain it in terms of the example app?

For instance, in the example app we have Customer.has_many :orders, and this is reflected in CustomerDashboard. I have tried to replicate the issue by adding this translation:

---
en:
  # ...
  helpers:
    label:
      order:
        address_state: "State code"

When I visit a customer show page, I do see the translation:

Customer dashboard showing the translation

Perhaps I'm looking at the wrong thing?

@jubilee2
Copy link
Contributor

@stormmaster42
Copy link
Contributor Author

stormmaster42 commented Aug 12, 2022

Oh yes. Sorry forgot to add that detail. Yes it's related to the active record fallback.
So if you declare the translation on attribute level like so:

en:
  activerecord:
    attributes:
      order:
        address_state: "State code"

it would be picked up by the orders dashboard, but not the embedded orders list in the customer show.

@pablobm
Copy link
Collaborator

pablobm commented Aug 15, 2022

Oh, I see! Thank you for explaining. The fact that resource_class can be either a method or a variable was confusing in particular. I get it now 👍

Don't worry about testing. This is too tricky (I already spent a lot of time trying to test something similar elsewhere and I'm not sure that there's much return of investment to be had). Merging.

@pablobm pablobm merged commit d5acc10 into thoughtbot:main Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants