Imo params should never be touched before they’ve gone through a changeset. It doesn’t need to be a schemas changeset and it doesn’t need persistance, but it should go through a changeset. Personally I even feel this can belong in the controller as opposed to in a context.

But actually doing validation at the incoming edge of the system AND doing it at the outgoing edge for data consistency in persistence is a lot of work and might seem unnecessary for as long as both validations essentially do the same, which they would for scaffold-ed code. As soon as you’re no longer passing params along “as is” to the changeset of your schema it’s no longer one and the same on both edges so it can be a solid solution to validate params directly in the controller already and let the schema’s changeset only deal with the data consistency as opposed to also deal with casting values.

Also I suggest treating the scaffolding of generators much more as a stepping stone then a holy grail. It’s meant go get people up to speed with phoenix quickly and show them high level concepts like not putting business logic into controllers, but it’s not an architecture you shouldn’t modify because it’s already perfect. There are tradeoffs being done just for the fact that code needs to be generated, there are tradeoffs between showing as few code as possible to not overwhelm people vs. showing a full picture. What even is a full picture to people of widely different degrees of experience generally and with phoenix specifically.

Hi @LostKobrakai! I don’t mean to highjack the thread, but now you got me curious…
I have to be honest and say that is quite “natural” given the way that the project is scaffolded to just patch the params since they are the common denominator in both controllers, contexts, and schemas. So I wonder what are you doing in cases where you want to use part of the data from the params and compute the rest of the fields in the program before they get cast.

I have another thread dedicated to this subject, would you mind sharing your experience there?

I’m gonna give you a practical example that came to my mind while reading your answer, picture this:

You have to insert a new user in the database and you must fill a required “role” field which is an enum in the database (standard, moderator, and administrator).

Although the names of the roles can be customized by each organization, they always will be represented by one of the valid values of the underlying enum type, so: padawan (standard), knight (moderator), and master (administrator). Because of this requirement, you have to fetch the values from the database using another module.

Now you have the following business rule to apply:

Two users can’t have the same role, the first user to request the role gets the role, the next one will be demoted automatically if the role is taken. This happens until every role is distributed to all users.

I see at least three things that must be done in order for this operation to succeed:

• Validate if the user input is valid (username, email, etc)
• Validate if the “role” field is a valid enum type (standard, moderator, and administrator)
• Validate if the “role” field does not break any constraints (unique role by organization)

Considering the default scaffolded code, I imagine that this means that in the contexts either you:

def create_user(params, organization_id) do
  role = Roles.valid_organization_role(organization_id)

  %User{}
  |> User.changeset(Map.put(params, :role, role))
  |> Repo.insert!
end

def create_user(params, organization_id) do
  %User{}
  |> User.changeset(params)
  |> fetch_and_validate_organization_role(organization_id)
  |> Repo.insert!
end

def fetch_and_validate_organization_role(changeset, organization_id) do
  role = Roles.valid_organization_role(organization_id)
  # using Ecto to validate the changeset and putting the changes in the changeset after?
end

def create_user(params, organization_id) do
  %User{}
  |> User.changeset(params, organization_id)
  |> Repo.insert!
end

defmodule User do
  
  def changeset(user, attrs, organization_id) do
   user
    |> cast(attrs, [:username, :email, :role])
    |> validate_required([:username, :email, :role])
    |> fetch_and_validate_organization_role(organization_id) 
  end
  
  def fetch_and_validate_organization_role(changeset, organization_id) do
    role = Roles.valid_organization_role(organization_id)
    # using Ecto to validate the changeset and putting the changes in the changeset after?
  end
end

def create_user(params, organization_id) do
  %User{}
  |> user_changeset(params, organization_id)
  |> Repo.insert!
end

def user_changeset(user, attrs, organization_id) do
  user
  |> cast(attrs, [:username, :email, :role])
  |> validate_required([:username, :email, :role])
  |> fetch_and_validate_organization_role(organization_id) 
end
  
def fetch_and_validate_organization_role(changeset, organization_id) do
  role = Roles.valid_organization_role(organization_id)
  # using Ecto to validate the changeset and putting the changes in the changeset after?
end

IMHO, I guess that this architectural “flexibility” is both the strong and weak suit of Phoenix as a framework. If I’m being honest I have questioned myself a lot about this before and I don’t think I’ve got a clear answer to that yet. In fact, if you take a look at the thread I linked in my previous post, you’ll see that there are some suggestions about not using changesets in the schema but directly in the contexts, which would allow you to cast fetch, compute and sanitize incoming data without breaking the “boundaries” of each “layer”.