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

Separate controller for each page resource #71

Open
smpallen99 opened this issue Jun 24, 2017 · 5 comments
Open

Separate controller for each page resource #71

smpallen99 opened this issue Jun 24, 2017 · 5 comments

Comments

@smpallen99
Copy link
Member

As I work though my first custom page on a project, I'm suspecting that every page will need controller customization. This is in contrast to resource controllers that controller customization might be the exception.

I don't think it makes sense to create some fancy API for handling controller customization for pages when we already have a controller generated. It makes more sense to work in the page controller directly.

I propose that we generate a page controller for each page (named after the page name) and generate the default action in the the controller. The default implementation of the action can be the same as the current page action in Talon.PageController. We can include some helpers in the Talon.PageController using block...

The way I'm going to handle customization in my project right now is

defmodule UcxUcc.Web.TalonPageController do
  use UcxUcc.Web, :controller
  use Talon.PageController, concern: UcxUcc.Admin

  plug Talon.Plug.LoadConcern, concern: UcxUcc.Admin, web_namespace: Web
  plug Talon.Plug.Theme
  plug Talon.Plug.Layout, layout: {Elixir.UcxUcc.Admin.AdminLte.Web.LayoutView, "app.html"}
  plug Talon.Plug.View

  def page(conn, params) do
    handle_page conn, params, Map.get(params, "page", "info")
  end

  def handle_page(conn, params, "info") do
   # My customization here
    render(conn, "info.html")
  end

  # render default page
  def handle_page(conn, params, page) do
    render(conn, "#{page}.html")
  end
end
@dsnider
Copy link
Member

dsnider commented Jun 24, 2017

I agree about using a page controller over a fancy API and making things easier by adding helpers to the Talon.PageController using block. We could either generate the default action in the target page controller or add a boilerplate comment for how to override.

@smpallen99
Copy link
Member Author

what about a separate page controller for each generated page? I really don't think it makes sense to generate 1 page controller that handles all pages.

@dsnider
Copy link
Member

dsnider commented Jun 25, 2017

I agree. With adding a controller along with the view and template, we have typical Phoenix. At that point, I'm not sure we should scope these under pages anymore. Just the resource name as the path and index as the action. These aren't really a lightweight pages anymore. The only difference between these and a backed resource are a few plugs and uses on the resource. I'm good with that.

If we go this direction, should we continue calling these pages or something else? If we call these something else, do we keep light weight page support? If so, do we use the single controller with multiple views approach, as we have now, or go with a single controller and a single view? I'm currently thinking the latter to support easily adding pages of straight templates. Or dropped them, for now.

Btw, I'm wondering if we should go ahead and do backed resources the same way.

@smpallen99
Copy link
Member Author

I can see ur point Dan. I'm not sure we need to make any distinction between light weight pages and something else. I see them as user controlled pages vs our CRUD pages. The name pages comes from ActiveAdmin. Personally, I feel "pages" is as good a name for them as anything else. Its short, succinct, and does not get confused with any other concept in Talon.

As for taking the same approach with resources, I'm not yet convinced we should take the same approach. I would suspect that a project will contain many more resource backed controllers than page controllers. Furthermore, in my experience, controller customization is required much less in resource backed controllers. And when it is needed, it may only be needed for a subset of the actions (at least that is my experience having built many projects using ExAdmin).

So, I still think a singular resource backed controller (per concern) works well. I may change my mind after more experience using Talon more...

In short, I believe adding code to page controllers is the norm, not the exception. And the opposite is true for resource backed controllers.

@odyright
Copy link

odyright commented Aug 7, 2017

@smpallen99 @dsnider nice work, I appreciated what you achieved. I just started to learn elixir, and I want to know something, with "talon" if the Admin want to create / register a customer/user himself, should we have to share the same "register_controller" between admin route & frontend route | or use a separated one? if yes how to specify the "conditional" code now?

I'm aware that in practice that isn't the case with ecommerce project...but it's most like educational systems with CRUD (where students can register themselves & the admin can register a student).

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

No branches or pull requests

3 participants