Create a `belongs_to` record from the associated record screen #1876

sdcoffey · 2023-07-31T16:49:15Z

Description

Allows creation of a belongs_to record from the new or edit pages of a resource.

Accomplished this by:

Adding a link to Create new x below the belongs_to field. This link points to the associated resource's new view with a special url param :via_belongs_to_resource_class.
Modifying BaseController#new to render a turbo stream adding a modal to the page containing the associated resource's new view, when the above param is present.
Modifying BaseController#create_success_action to, when the above param is present, render two turbo streams, one removing the modal, and the other containing a custom event with the details of the new record.
Adding a new stimulus controller to handle the custom event, updating the field with the params from the newly created record. Aside: although this introduces some bespoke js, it turned out to be way less intrusive than some other, non client-side approaches I tried.

Fixes #1556

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added tests that prove my fix is effective or that my feature works

Manual review steps

Create a Course from edit/new of Course::Link - searchable belongs_to association
Create a User from edit/new of Fish - belongs_to with normal HTML select
Create a Post or Project from new/edit of Comment - polymorphic belongs_to
Create a Fish from new/edit of Review - searchable, polymorphic belongs_to

Manual reviewer: please leave a comment with output from the test if that's the case.

Open Questions

Should there be a field-level option to enable/disable this behavior?
How should we handle cases where the resource pointed to is an abbreviated version of the associated resource (e.g. CompactUser)?

…elationship

…ontroller

app/javascript/js/controllers/fields/reload_belongs_to_field_controller.js

codeclimate · 2023-07-31T16:49:49Z

Code Climate has analyzed commit 528432c and detected 0 issues on this pull request.

View more on Code Climate.

github-actions · 2023-08-17T01:53:19Z

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

adrianthedev · 2023-08-19T12:49:04Z

@Paul-Bob can you please also have a look over this when you get a chance? Thank you.

Paul-Bob · 2023-08-23T06:11:48Z

Hi @sdcoffey, first of all thank you for the amazing PR.

I've been testing it and came across a few issues. The modal that opens for the belongs_to creation seems a little unpolished, for example, when creating an user or some record that have a big resource it expands to full height, maybe we should assure some margins. It's no biggie.

My main concern revolves around the form errors. They no longer seem to show up when submitting a form and some validation fails. It happens due turbo-stream I think. We should focus on solving this error rendering issue first.

Otherwise, the functionality seems to be there, good job!

Paul-Bob

Let's assure that errors render on the form when validations fails

Paul-Bob · 2023-08-23T06:22:24Z

app/views/avo/partials/_keep_modal_open.turbo_stream.erb

+<%= turbo_stream.append "alerts" do %>
+  <%= render Avo::FlashAlertsComponent.new flashes: flash %>
+<% end %>


Maybe we should rename this stream to .../_flash_alerts.turbo_stream.erb I think it fits better to the file porpoise.

sdcoffey · 2023-08-25T18:40:47Z

Thanks for the review @Paul-Bob! Will take another look at the issue with large resources and fix the missing validation errors (totally missed that).

Will ping you again when I've finished updating

…ide modal

sdcoffey · 2023-09-11T14:58:29Z

Hey all! Been a couple weeks since the last activity, wanted to bump this to see about getting it across the finish line

adrianthedev · 2023-09-11T18:42:06Z

Yes @sdcoffey. You're right! It's on me to review it.
Sorry about the delay.

I'll give it a go tomorrow.

sdcoffey · 2023-09-11T19:08:15Z

No worries, just wanted to make sure it didn't get lost–I'm sure y'all are busy! Thank you! 🙏

adrianthedev · 2023-09-15T11:38:47Z

@sdcoffey it works great but I need to check the code a bit and the styling. It's on my plate for the weekend.

sdcoffey · 2023-09-16T00:04:07Z

Sounds good! Thank for the update

adrianthedev · 2023-10-20T16:27:43Z

Thanks @sdcoffey for the contribution.
Sorry it took so long to get it merged.

sdcoffey · 2023-10-20T16:35:53Z

🙏 No worries! Thanks for the cleanup.

Excited to see this make it into the next release!

adrianthedev · 2023-10-20T16:36:36Z

Yes. It will be out on Tuesday 🙌

segiddins · 2023-10-24T16:33:11Z

app/components/avo/modal_component.html.erb

+          </div>
+        <% end %>
+
+        <% if content? %>


I think this content? call is causing a NoMethodError

@segiddins can you describe the scenario? I wasn't able to reproduce it

rubygems/rubygems.org#4154 yeah, tests started failing on the version bump

I think you might need to update view_component on your end.
I can't put my finger on it exactly, but at some point they changed the way they figure out if the slot has any content inside.

I see that view_component v3 added this helper. Are you able to update to VC 3 @segiddins?
I see that you are already on 2.8+ which should not bring breaking changes AFAIK.

xeron · 2023-10-25T03:45:40Z

I'm getting a error trying to use new resource links introduced by this PR.

e.g. this URL /admin/resources/drivers/new?via_belongs_to_resource_class=ResultResource&via_relation=driver gives me a error:

No route matches {:action=>"new", :controller=>"avo/drivers"}

but this works just fine /admin/resources/drivers/new.

Trace example:

Started GET "/admin/resources/drivers/new?via_belongs_to_resource_class=ResultResource&via_relation=driver" for 127.0.0.1 at 2023-10-24 20:48:43 -0700
Processing by Avo::DriversController#new as HTML
  Parameters: {"via_belongs_to_resource_class"=>"ResultResource", "via_relation"=>"driver"}
  Rendered vendor/bundle/ruby/3.2.0/gems/avo-2.43.0/app/views/avo/base/_new_via_belongs_to.html.erb (Duration: 13.3ms | Allocations: 8543)
Completed 500 Internal Server Error in 18ms (ActiveRecord: 0.0ms | Allocations: 10221)

ActionView::Template::Error (No route matches {:action=>"new", :controller=>"avo/drivers"}):
    1: <%= turbo_frame_tag "new_via_belongs_to" do %>
    2:   <%= render(Avo::ModalComponent.new(width: :xl, body_class: "bg-application")) do |c| %>
    3:     <div class="pt-4 pb-8">
    4:       <%= render Avo::Views::ResourceEditComponent.new(
    5:         resource: @resource,
    6:         model: @model,
    7:         view: @view,

xeron · 2023-10-26T01:53:43Z

@sdcoffey could you please take a look? ^^ It doesn't open a modal for me, it just goes to the link and shows a "No route" error.

adrianthedev · 2023-10-31T19:20:04Z

@xeron was this fixed for you? I can't replicate it on my end.
If you can share a reproduction repository I'll gladly have a look.

xeron · 2023-10-31T20:33:38Z

No, I tried to remove all my avo customizations:

custom avo stimulus controllers
custom avo _head and _scripts partials
custom avo routes

also tried downgrading rails from 7.1 to 7.0.

And it still didn't work. Clicking Create new resource just opens a page with a routing error for me.

I'll try to create a minimal app which reproduces the issue.

adrianthedev · 2023-10-31T20:36:20Z

Can you check if you have a JS error somewhere. To me that sounds like the JS is not working. Or Turbo doesn't execute it as a Turbo request.
That should be a turbo request.

xeron · 2023-10-31T20:52:12Z

Can you check if you have a JS error somewhere. To me that sounds like the JS is not working. Or Turbo doesn't execute it as a Turbo request.

This was the 1st thing I did when I faced the issue :)

Anyway I think I found the problem.

Enabling the following in config/initializers/avo.rb breaks the functionality:

config.buttons_on_form_footers = true

xeron · 2023-11-01T00:10:37Z

I've reported it as an issue for tracking purposes: #2008

adrianthedev · 2023-11-01T10:32:37Z

Pfff. What a weird bug!
Thanks for looking into this.
I'll add it to the roadmap.

Paul-Bob · 2023-11-09T16:23:43Z

Can you check if you have a JS error somewhere. To me that sounds like the JS is not working. Or Turbo doesn't execute it as a Turbo request.

This was the 1st thing I did when I faced the issue :)

Anyway I think I found the problem.

Enabling the following in config/initializers/avo.rb breaks the functionality:
config.buttons_on_form_footers = true

Back button without valid path was rendering on the form footer, this PR fixes it. Thanks for pointing to the right point and for building a detailed issue!

lpe · 2023-11-12T13:55:33Z

It would indeed be important to be able to disconnect this option, if possible at field level. For example I have "Task belongs_to User", I can pick a User (searchable). This certainly does not mean that I can create a User. Maybe there's another way to disable it? Thx.

xeron · 2023-11-12T17:08:51Z

It would indeed be important to be able to disconnect this option

I was thinking about the same. This feature should be opt-in or opt-out in belongs_to field settings.

In fact I even had this in my code before:

field :driver, as: :belongs_to,
  help: 'You can add new drivers <a href="/admin/resources/drivers/new" target="_blank">here</a>',

So this feature automatically added duplicate functionality to some of my fields.

adrianthedev · 2023-11-13T14:13:08Z

Yup. That makes sense.
Apologies for the troubles.
We'll add a mechanism to hide that option.

sdcoffey added 9 commits July 29, 2023 11:25

wip

26126ad

move keep_modal_open view to shared partial

d292661

update base controller to show modal when creating via a belongs_to r…

b9c7b9e

…elationship

WIP add create button to belongs_to edit field with return-handling c…

b7754fd

…ontroller

hook up panel back button from modal

afc20c3

pass params[:via_belongs_to_resource_class] through #create

c3a958b

add WIP system spec for belongs_to cases

c913b65

update specs to cover polymorphic and searchable polymorphic belongs_to

6b3effb

style and cleanup

6336054

codeclimate bot reviewed Jul 31, 2023

View reviewed changes

app/javascript/js/controllers/fields/reload_belongs_to_field_controller.js Show resolved Hide resolved

sdcoffey added 2 commits July 31, 2023 10:00

decompose long method in reload_belongs_to_controller

f68c175

fixup! decompose long method in reload_belongs_to_controller

368533b

sdcoffey changed the title ~~🚧 WIP - Create a belongs_to record from the associated record screen~~ Create a belongs_to record from the associated record screen Jul 31, 2023

remove change to .gitignore

0454a34

sdcoffey mentioned this pull request Jul 31, 2023

Create a belongs_to record from the associated record screen #1556

Closed

remove usage of class_names

25f91b6

github-actions bot added the Stale label Aug 17, 2023

adrianthedev added Stale exempt and removed Stale labels Aug 18, 2023

adrianthedev requested a review from Paul-Bob August 19, 2023 12:48

Paul-Bob requested changes Aug 23, 2023

View reviewed changes

Paul-Bob reviewed Aug 23, 2023

View reviewed changes

sdcoffey added 3 commits August 25, 2023 11:44

rename _keep_modal_open -> _flash_alerts

1e44d70

ensure validation errors are still shown after create failure

642136a

add in_modal param to PanelComponent to allow better form display ins…

540a92b

…ide modal

Paul-Bob approved these changes Aug 30, 2023

View reviewed changes

tweaks

8427405

adrianthedev approved these changes Oct 20, 2023

View reviewed changes

adrianthedev merged commit 11aae5c into avo-hq:main Oct 20, 2023
10 checks passed

Paul-Bob added Feature and removed Stale exempt labels Oct 24, 2023

segiddins reviewed Oct 24, 2023

View reviewed changes

xeron mentioned this pull request Nov 1, 2023

config.buttons_on_form_footers breaks new belongs_to record functionality #2008

Closed

10 tasks

This was referenced Nov 15, 2023

Hide the ability to add a belongs to from the parent record #2033

Closed

Create new record from belongs_to associations crashes the app #2045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a `belongs_to` record from the associated record screen #1876

Create a `belongs_to` record from the associated record screen #1876

sdcoffey commented Jul 31, 2023 •

edited

Loading

codeclimate bot commented Jul 31, 2023 •

edited

Loading

github-actions bot commented Aug 17, 2023

adrianthedev commented Aug 19, 2023

Paul-Bob commented Aug 23, 2023 •

edited

Loading

Paul-Bob left a comment •

edited

Loading

Paul-Bob Aug 23, 2023 •

edited

Loading

sdcoffey commented Aug 25, 2023

sdcoffey commented Sep 11, 2023

adrianthedev commented Sep 11, 2023

sdcoffey commented Sep 11, 2023

adrianthedev commented Sep 15, 2023

sdcoffey commented Sep 16, 2023

adrianthedev commented Oct 20, 2023

sdcoffey commented Oct 20, 2023

adrianthedev commented Oct 20, 2023

segiddins Oct 24, 2023

Paul-Bob Oct 24, 2023

segiddins Oct 24, 2023

adrianthedev Oct 24, 2023

adrianthedev Oct 24, 2023

adrianthedev Oct 24, 2023 •

edited

Loading

xeron commented Oct 25, 2023 •

edited

Loading

xeron commented Oct 26, 2023

adrianthedev commented Oct 31, 2023

xeron commented Oct 31, 2023 •

edited

Loading

adrianthedev commented Oct 31, 2023

xeron commented Oct 31, 2023 •

edited

Loading

xeron commented Nov 1, 2023

adrianthedev commented Nov 1, 2023

Paul-Bob commented Nov 9, 2023

lpe commented Nov 12, 2023

xeron commented Nov 12, 2023 •

edited

Loading

adrianthedev commented Nov 13, 2023

Create a belongs_to record from the associated record screen #1876

Create a belongs_to record from the associated record screen #1876

Conversation

sdcoffey commented Jul 31, 2023 • edited Loading

Description

Checklist:

Manual review steps

Open Questions

codeclimate bot commented Jul 31, 2023 • edited Loading

github-actions bot commented Aug 17, 2023

adrianthedev commented Aug 19, 2023

Paul-Bob commented Aug 23, 2023 • edited Loading

Paul-Bob left a comment • edited Loading

Choose a reason for hiding this comment

Paul-Bob Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

sdcoffey commented Aug 25, 2023

sdcoffey commented Sep 11, 2023

adrianthedev commented Sep 11, 2023

sdcoffey commented Sep 11, 2023

adrianthedev commented Sep 15, 2023

sdcoffey commented Sep 16, 2023

adrianthedev commented Oct 20, 2023

sdcoffey commented Oct 20, 2023

adrianthedev commented Oct 20, 2023

segiddins Oct 24, 2023

Choose a reason for hiding this comment

Paul-Bob Oct 24, 2023

Choose a reason for hiding this comment

segiddins Oct 24, 2023

Choose a reason for hiding this comment

adrianthedev Oct 24, 2023

Choose a reason for hiding this comment

adrianthedev Oct 24, 2023

Choose a reason for hiding this comment

adrianthedev Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

xeron commented Oct 25, 2023 • edited Loading

xeron commented Oct 26, 2023

adrianthedev commented Oct 31, 2023

xeron commented Oct 31, 2023 • edited Loading

adrianthedev commented Oct 31, 2023

xeron commented Oct 31, 2023 • edited Loading

xeron commented Nov 1, 2023

adrianthedev commented Nov 1, 2023

Paul-Bob commented Nov 9, 2023

lpe commented Nov 12, 2023

xeron commented Nov 12, 2023 • edited Loading

adrianthedev commented Nov 13, 2023

Create a `belongs_to` record from the associated record screen #1876

Create a `belongs_to` record from the associated record screen #1876

sdcoffey commented Jul 31, 2023 •

edited

Loading

codeclimate bot commented Jul 31, 2023 •

edited

Loading

Paul-Bob commented Aug 23, 2023 •

edited

Loading

Paul-Bob left a comment •

edited

Loading

Paul-Bob Aug 23, 2023 •

edited

Loading

adrianthedev Oct 24, 2023 •

edited

Loading

xeron commented Oct 25, 2023 •

edited

Loading

xeron commented Oct 31, 2023 •

edited

Loading

xeron commented Oct 31, 2023 •

edited

Loading

xeron commented Nov 12, 2023 •

edited

Loading