-
Notifications
You must be signed in to change notification settings - Fork 61
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
Update documentation for HashPasswordChange #842
base: main
Are you sure you want to change the base?
Conversation
Provide clearer pseudo-code, which reflects the removal of Ash.Changeset.new/2 and shows the interaction between arguments and the `change` in the Resource.DSL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @LannyBose 👋
Thanks for the contribution. Unfortunately they're not quite right. If you can update your PR I'll be happy to merge it, otherwise I'll swing back in a day or two and make the changes myself.
Changeset.new(user, %{}) | ||
|> Changeset.set_context(%{strategy_name: :password}) | ||
|> Changeset.for_update(:update, params) | ||
|> Accounts.update() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that Ash.Changeset.new/2
is gone, however the rest of your example is incorrect.
It should probably be more like:
Changeset.new(user)
|> Changeset.set_context(%{strategy_name: :password})
|> Changeset.for_update(:change_password, params)
|> Accounts.update()
@@ -30,6 +32,8 @@ defmodule AshAuthentication.Strategy.Password.HashPasswordChange do | |||
|
|||
```elixir | |||
update :change_password do | |||
argument :your_unencrypted_password_field, :string, sensitive?: true | |||
|
|||
change {AshAuthentication.Strategy.Password.HashPasswordChange, strategy_name: :password} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that these examples should be updated to match the action that the add strategy igniter generates.
When referencing how to make a manual password change, I felt the included documentation could use some sprucing up.
Changes include:
Ash.Changeset.new/2
seems to be goneset_context
didn't work as expected when handed toHashPasswordChange.change
. It kept resulting inBadMapError
update
Resource DSLIt's very possible my first chunk of code isn't idiomatic, but it did work. Happy to receive adjusting feedback there!