Removes ja_resource dependency from OrganizationInviteController #1051
Conversation
joshsmith
force-pushed
the
894-remove-ja-resource-from-organization-invite-controller
branch
from
9488d23 to
8f389c0
Compare
joshsmith
force-pushed
the
894-remove-ja-resource-from-organization-invite-controller
branch
from
8f389c0 to
ced89bc
Compare
There was a problem hiding this comment.
Great work here! I hope you don't mind that I fixed the merge conflict, in large part because I was the reviewer for the files that caused the conflict.
I pointed out some other things here that I fixed along the way for future reference, so you can still get the benefit of a (hopefully helpful) code review without the explicit need to change it yourself.
|
|
||
| @spec model :: module | ||
| def model, do: CodeCorps.OrganizationInvite |
|
|
||
| import CodeCorps.Helpers.Query, only: [id_filter: 2] |
There was a problem hiding this comment.
This was no longer in use since you're using Helpers.Query.id_filter/1 directly in the show/2.
| {:ok, :authorized} <- current_user |> Policy.authorize(:create, %OrganizationInvite{}, params), | ||
| {:ok, %OrganizationInvite{} = organization_invite} <- %OrganizationInvite{} |> OrganizationInvite.create_changeset(params) |> Repo.insert do | ||
|
|
||
| send_email(organization_invite) |
There was a problem hiding this comment.
There was a merge conflict due to a recent merge. The rebase made the changeset really weird so I had to come in and do this manually. You'll see below that render_create/2 was removed, but I kept the private send_email/1 and opted not to handle it within the with but inside the result, where I think the async process makes more sense.
|
@lkoller 🙌 on this. I invited you as a member – you should be able to view the invitation here – so you can merge at will! Please do stop by our Slack and say hi: http://slack.codecorps.org/ |
|
Just a heads up that I've since merged something else in, so you'll need to do a |
…invite-controller
|
Great work @lkoller I went ahead and merged this for you, so I can have an easier time merging other branches. |
|
Thank you so much for the great feedback! Excited to work on a few more of these too |
lkoller
deleted the
894-remove-ja-resource-from-organization-invite-controller
branch
What's in this PR?
This PR removes the ja_resource from the OrganizationInviteController
References
Fixes #894
Progress on: #864