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

Fixes for Blog.Accounts.create_user/1 #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

smaximov
Copy link

@smaximov smaximov commented Mar 5, 2018

Fix transaction in Blog.Accounts.create_user/1

If Blog.Accounts.create_user/1 is provided with data which doesn't pass the validations in Blog.Account.{User,Contact}.changeset/2, the lambda function passed to Ecto.Repo.transaction/2 will return {error, %Ecto.Changeset{valid?: false}}. This will not automatically roll back the transaction, as transaction are rolled back only if an error is raised, as mentioned in the docs:

If an unhandled error occurs the transaction will be rolled back and the error will bubble up from the transaction function. If no error occurred the transaction will be committed when the function returns. A transaction can be explicitly rolled back by calling rollback/1, this will immediately leave the function and return the value given to rollback as {:error, value}

Not only it results in stale records in the database, it messes the return value of Blog.Accounts.create_user/1 as well. Because Ecto.Repo.transaction/2 returns the value returned by the function wrapped in a tuple as {:ok, value}, in case of validation errors it will return {:ok, {:error, %Ecto.Changeset{valid?: false}}}.

To fix both issues, we need to match on {:error, changeset} and manually roll back the transaction.

Validate uniqueness of (type, value) for the contacts table

contacts table has a unique constraint on (type, value). If the user tries to insert the same contact twice, an error will be raised.

@smaximov smaximov changed the title Fix transaction in Blog.Accounts.create_user/1 Fixes for Blog.Accounts.create_user/1 Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant