wfth / wo

0 stars 0 forks source link

Fix Admin side routing #29

Closed isaacjwilliams closed 7 years ago

isaacjwilliams commented 7 years ago

Implementing Phoenix.Param on Sermon Series and Sermons so we could slugify params had unexpected consequences. All the link helpers on the Admin side now generate slug routes, which has completely mangled that side of the site.

I'm not sure what we should do about it. I spent some time trying to fix it, but none of my solutions (passing ids instead of structs to the helpers, creating a new model for Sermon Series, using slug routes, +++) are satisfactory. We need to look over this together @aiwilliams !

aiwilliams commented 7 years ago

I think this requires some research.

Ultimately we need to understand how Phoenix works to call up on the to_params function so that we can learn whether there is a way to limit the scope of that work to specific controllers/Helpers.

isaacjwilliams commented 7 years ago

I created a test app using Phoenix 1.3 and I can confirm with 100% certainty: parameterization works exactly the same.

isaacjwilliams commented 7 years ago

One inkling of an idea: maintain an admin context and a visitor context.

Chris McCord and José Valim have bought into the bounded context pattern from Domain-Driven Design. Models have been replaced by what Phoenix 1.3 calls "contexts". So to generate a blog context with a comments module, you run mix phx.gen.html Blog Comments comment title:string content:string which generates something like this:

|-- lib
    |-- my_app
        |-- blog
            |-- comment.ex

In the Domain-Driven Design book, Eric Evans says that one indication of context splintering is a false cognate, a single implemented object that is used in two different ways. I think that what we're seeing with this to_param issue may be a symptom of a false cognate.

I suggest that our structure look something like this:

wo-contexts

Or, in the Phoenix 1.3 directory structure, like this:

|-- lib
    |-- wo
        |-- admin
            |-- sermon_series.ex
            |-- sermon.ex
        |-- visitor
            |-- sermon_series.ex
            |-- sermon.ex
aiwilliams commented 7 years ago

I think this generally makes great sense. There will be challenges around:

  1. a feeling of duplication where two schemas reference a single table
  2. wonder at when to create a new context
  3. are contexts role based or activity based or ??

In our conversation, I think we agreed to:

aiwilliams commented 7 years ago

Once you have upgraded Phoenix and have the different ecto types(?), the to_param-defimpl-with-our-type thingy can be defined for the content browser ecto type to do the slug urls 🏅

aiwilliams commented 7 years ago

Working together, we learned:

What remains is working through all the controllers to use the bounded context interfaces.

isaacjwilliams commented 7 years ago

Using a beautiful method developed by @aiwilliams, we have solved this issue. Blogpost about it coming soon(ish)!