Add password strategy compatible with simple-ember-auth : refactor branch

[alexjp]

Is the branch refactor the correct branch?

Rewrote changes from scratch, what is currently working :

• added registration by username ( email still works as before, all tests pass )
• when registering by username, does not send email and auto confirms
• allow to create session by username, sends the reply that simple ember auth expects

I think this patch is better and cleaner, although, the function names/if's could be better.

[manukall]

thanks! really happy you're contributing.

i'm gonna add a few minor comments to the files, but i like the general concept! :+1: :+1: :+1:

[manukall]

oh tests for the new functionality would also be greatly appreciated.

[alexjp]

tried to do all your notes. did not manage to make (~(username) -> ~(email username)) set at the same time, but made it better than i had.

if everything is ok, tests are coming :)

[alexjp]

Added some tests. I am having trouble with my "elixir", The registrator signature is expecting a map, but the login test is sending structs. That fails the last two login tests.

What is the best way around this ?

[manukall]

that should actually work, as structs are maps. are the keys different? maybe atoms instead of strings?

[alexjp]

fixed the tests :)

[alexjp]

Added the user id into the session response. This should make it alot easier to get the user id.

Previously, it needed to:

• do login
• do request to account
• do request to proper user model with query based on email or username

now should be:

• do login
• do request to proper user model, fetching the proper resource by id

The account is nice, but there is two problems for me:

• not big deal, but would have to create an account model just to store the username and email
• ( very important ) would have to format the account response according to jsonapi for ember

[manukall]

Review status: 0 of 11 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

^{lib/phoenix_token_auth/mailer.ex, line 0 [r4] (raw file):} i think using functions instead of conditions is indeed preferred in elixir. that's why i would suggest the approach of using two implementations of UserController#create. I imagined something like:

  def create(conn, %{"user" => %{email: email}}) when email != ""... do
      {confirmation_token, changeset} = Registrator.changeset(params)
      |> Confirmator.confirmation_needed_changeset

    if changeset.valid? do
      case Util.repo.transaction fn ->
        user = Util.repo.insert(changeset)
             Mailer.send_welcome_email(user, confirmation_token)
      end do
        {:ok, _} -> json conn, :ok
      end
    else
      Util.send_error(conn, Enum.into(changeset.errors, %{}))
    end

    def create(conn, %{"user" => params}) do
        changeset = Registrator.changeset(params)

    if changeset.valid? do
      case Util.repo.transaction fn ->
        user = Util.repo.insert(changeset)
        end do
        {:ok, _} -> json conn, :ok
      end
    else
      Util.send_error(conn, Enum.into(changeset.errors, %{}))
    end
  end

( note: this is only badly formatted pseudo code)

the reason i didn't like the two send_email method implmementations was just that i think an error should be raised at that point if no email is present. in the controller we know that there are two kinds of creating a user and handle that: one with email and one with password. we should try to keep that logic somewhat local there, so we do not accidently swallow errors in the mailers that are real errors.

---

Comments from the review on Reviewable.io

[alexjp]

Understood... i tried doing something like that but saw that i was duplicating alot of code just because an if.

I will update the code accordingly. thanks !

[manukall]

hey @alexjp, what's the state here? there is one failing test and lot's of deprecation warnings...

[alexjp]

Sorry, it was because i updated ecto version, and insert/2 and update/2 are now insert!/2 and update!/2.

I have been away from computer there last few days, but will update it accordingly.

[alexjp]

@manukall very lots of sorry for the delay, should be all fixed.

[manukall]

@alexjp thanks again! great work!!! i'll merge the refactor branch into master and release a new version with your changes now...

[alexjp]

really, thank you @manukall , for phoenix_token_auth and the help in this pull request.

I will try to post online in the weekend, an example of server api in phoenix and web app in emberjs. It could be included in the readme, for help references.

The app actually is already done, only missing removing my repository and moving it to this one.

With this and jsonapi, i figure elixir and phoenix opens alot of doors for emberjs developers!!!

thanks !!!!!!