Remove JaResource/Canary from SkillController

[landongrindheim]

Remove SkillController's reliance on JaResource/Canary.

References

Closes #903

Progress on: #864 (working toward closing this one!)

[landongrindheim]

I wasn't able to find a community standard for a with clause which uses piping. This felt right, but I'm open to other suggestions 😄

[begedin]

Simplest trick I can think off would be to add a defp load_skills(%{} = params) into the controller and call

with skills <- params |> load_skills() do

[landongrindheim]

I also considered these options:

  # 1)
  def index(%Conn{} = conn, %{} = params) do
    with skills <- Skill |> id_filter(params) |> title_filter(params) |> limit_filter(params) |> Repo.all do
      conn |> render("index.json-api", data: skills)
    end
  end

  # 2)
  def index(%Conn{} = conn, %{} = params) do
    with skills <- Skill
         |> id_filter(params)
         |> title_filter(params)
         |> limit_filter(params)
         |> Repo.all
    do
      conn |> render("index.json-api", data: skills)
    end
  end

  # 3)
  def index(%Conn{} = conn, %{} = params) do
    with skills <- Skill
         |> Query.id_filter(params)
         |> Query.title_filter(params)
         |> Query.limit_filter(params)
         |> Repo.all,
      do: conn |> render("index.json-api", data: skills)
  end

  # 4)
  def index(%Conn{} = conn, %{} = params) do
    with skills <-
      Skill
      |> Query.id_filter(params)
      |> Query.title_filter(params)
      |> Query.limit_filter(params)
      |> Repo.all,
      do: conn |> render("index.json-api", data: skills)
  end

[begedin]

Overall looks good, just a couple minor tweaks to make it easier to read.

[begedin]

I think this just adds an extra layer we don't really need. I see why you did it, but I think it would be preferable to revert on this and instead just call Skill.changeset from the controller.

[begedin]

Simplest trick I can think off would be to add a defp load_skills(%{} = params) into the controller and call

with skills <- params |> load_skills() do

[landongrindheim]

@begedin Thanks for the feedback! Can you let me know if this lines up with what you're thinking? If so, I'll squash the commits

[begedin]

Great work 👍