bullet-train-pro / bullet_train-action_models

Other
5 stars 1 forks source link

Build path for action models dynamically when namespace has a resource #94

Closed gazayas closed 1 year ago

gazayas commented 1 year ago

Closes #93. This PR also assumes the routes setup spelled out in this comment.

The Problem

Actions can be namespaced like this: Listings::PublishAction. However, when the routes are deeply nested and parents in the hierarchy are resources, we don't get the proper routes.

In the model setup in #93, we have the following:

Newsletters::Issues::SendAction

Super scaffolding generates links to urls with arrays like this:

# Passed to `link_to`, etc., and generates a path for us
[:account, send_action]

This would produce a path like this, because the send_action class is Newsletters::Issues::SendAction:

account_newsletters_issues_send_action_path(send_action)

This wasn't working because newsletters is a resource, and [:account, send_action] has no knowledge of the newsletter resource, raising a NoMethodError. What we really need is this:

account_newsletter_issues_send_action_path(newsletter, send_action)

It should be noted that in some places, the parent resource was being scaffolded correctly for collection actions. For example, in #93 I brought up this code:

<%= button_to "Send", [:new, :account, newsletter, :issues_send_action], method: :get, class: 'button-secondary', form: {class: 'button_to'} %>

The link here works for collection actions, but when we need both the newsletter resource and the send_action resources, we weren't scaffolding the proper routes, and it just showed up in different places like [:account, send_action].

The Solution

What this PR does is, it takes the current url path and builds the action model path based off of that. So it does the following:

  1. Takes the current url
  2. Reads each part of the url and determines if it's a resource or just a namespace
  3. Registers the object of each resource in the path to eventually pass as arguments
  4. Builds the path_method, then passes the arguments we got in step 3

So ultimately we're calling this: send(path_method, *object_arguments) Which in our example results in this:

account_newsletter_issues_send_action_path(newsletter, send_action)

The reason for using the requests url is that I'm assuming we only want these links when we're on a page in the namespace. For example, we wouldn't call the action from another random resource in the application.

Implementation

In our views like _action.html.erb in bullet_train-themes-light, we would do the following:

<% action_model_path = build_action_model_path(request, action) %>
<%= link_to t("#{action.class.name.pluralize.underscore}.buttons.shorthand.show"), action_model_path, class: 'button-secondary button-smaller' %>

I will have to make joint PR which should include the following:

  1. Replacement of url arrays with backwards compatibility
  2. Updates to Super Scaffolding to scaffold this helper instead of arrays like [:account, send_action]
  3. Potentially add documentation. I don't think this will affect developers with actions they currently have in place because we're first checking with this:
    def build_action_model_path(request, action, type: nil)
    url_for([:account, action])
    rescue NoMethodError
    ...
    end

If we need documentation to explain what's going on though, I can write that.

gazayas commented 1 year ago

Ok, this takes care of the path issue. I stopped using the URL to determine what path we want because I figured it was a bad idea, and the idea to check the *_id attributes came to me this morning after I took a step away from this PR for a bit.

We still have to add the routes manually (from this comment. I plan on working in the RoutesFileManipulator here next), but as far as the links themselves, I think the work on this one is done for target-one-parent actions.

You will also have to add this code from bullet_train-core.

Other actions

I haven't tested this with other actions, but I could see the same issue happening. For now, I'll just keep the changes for the targets-one-parent action.

Multiple calls to the database

The only thing I don't like about this is that we're making calls to the database multiple times if we use the same helper method more than once in a view. It's not the end of the world, just something to consider moving forward.