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

Apply link_to_child_resource on associations. #2651

Closed
Paul-Bob opened this issue Apr 1, 2024 · 12 comments
Closed

Apply link_to_child_resource on associations. #2651

Paul-Bob opened this issue Apr 1, 2024 · 12 comments
Assignees

Comments

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Apr 1, 2024

Context

From here: https://discord.com/channels/740892036978442260/1201874761546735636/1223388242917851176

link_to_child_resource option should also apply on associations.

Copy link
Contributor

This issue has been marked as stale because there was no activity for the past 15 days.

@cyu
Copy link

cyu commented May 18, 2024

Any progress on this issue? The documentation seems to imply that this should work: https://docs.avohq.io/3.0/associations.html#link-to-child-resource-when-using-sti

@adrianthedev
Copy link
Collaborator

Not really, sorry about that.
We'll try to prioritize it in the next few weeks.

Do you think you could lend a hand and maybe check if you could submit a PR?
That would definitely go faster.

Thank you!

@MrJoy
Copy link
Contributor

MrJoy commented Aug 10, 2024

@adrianthedev Doing some digging into the code, here's what I've discovered.

When deciding on the link to the record, the determination of whether to link to a child resource is made based on the :id field. That is of type IdField which doesn't support link_to_child_resource, and so the field_linked_to_child_resource? is returning false.

There is some sort of pseudo-field being called for the association itself, which is where the link_to_child_resource: on field(:my_association, as: :has_many, link_to_child_resource: <whatever>) is relevant. It's just that the determination of whether to refer to the child resource made here is ignored.

@MrJoy
Copy link
Contributor

MrJoy commented Aug 10, 2024

It looks like the edit/show buttons are using the association "field" to determine whether to link to the child resource. So, they obey link_to_child_resource on the field defining the association.

So it comes down to: What is the expected behavior of IdField when rendering index as an association?

It seems to me it should be punting over to that association to make the determination, but I feel like this is... going to get a little gnarly logic-wise and am hesitant to try and hack something together for it. I'll take a quick stab and see what I come up with.

@MrJoy
Copy link
Contributor

MrJoy commented Aug 10, 2024

@adrianthedev It seems to me -- and I of course could be wildly misunderstanding things -- that basically in parent_or_child_resource, you need to change the logic when field.present? && field.is_a?(Avo::Fields::IdField) && @parent_record.present?.

If that condition is true, you need to consult a different field. Specifically, the "field" that's being consulted for rendering the show/edit buttons.

And the behavior of that is... kinda weird. link_to_child_resource defined on the association field is only obeyed when it's false. If we set self.link_to_child_resource = false on the resource, and link_to_child_resource: true on the association field, the row controls will prefer to listen to the resource.

It seems to me more flexible / less surprising behavior would be to list to link_to_child_resource on the association field if it's not nil, and fall back to the value defined on the resource otherwise.

My hangup in contributing a PR is that I don't know how to get access to that association field object from parent_or_child_resource, when it's called in the context of an IdField...

@MrJoy
Copy link
Contributor

MrJoy commented Aug 10, 2024

@adrianthedev Ok. So. This is... more or less Doing The Thing in my testing. I'm 100% sure that it's not the right way to look up the field object though, because I'm doing some monkey-patching that renames field to raw_field and creates a new field method that basically just calls with_options(...) { raw_field(...) } and for this one specific association I get back a TextField with the following code. Doesn't seem to happen for other has_many associations on the resource! No idea why. So, whatever method is used to look up the relevant association field when render_row_controls is invoked is gonna be relevant.

  def parent_or_child_resource
    if field.present? && field.is_a?(Avo::Fields::IdField) && @parent_record.present?
      parent_field = parent_resource.fields_by_database_id[params["related_name"]]
      if parent_field.link_to_child_resource
        return Avo.resource_manager.get_resource_by_model_class(@resource.record.class)
      end
    end

    return @resource unless link_to_child_resource_is_enabled?
    return @resource if @resource.record.class.base_class == @resource.record.class

    Avo.resource_manager.get_resource_by_model_class(@resource.record.class) || @resource
  end

@Paul-Bob Paul-Bob self-assigned this Aug 13, 2024
@Paul-Bob
Copy link
Contributor Author

Paul-Bob commented Aug 14, 2024

Hey @MrJoy I guess this PR might fix this issue. Can you provide any reproduction repository where I can confirm it (or confirm it yourself if it's faster / easier)?

@Paul-Bob
Copy link
Contributor Author

@MrJoy can you please verify if the issue persists on avo 3.11.7?

@MrJoy
Copy link
Contributor

MrJoy commented Aug 19, 2024

@Paul-Bob Sorry for the delay! Will take a look now!

@MrJoy
Copy link
Contributor

MrJoy commented Aug 19, 2024

@Paul-Bob This does indeed appear fixed! TY!

@Paul-Bob
Copy link
Contributor Author

Great news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants