Refactor matches

[mononoken]

🔗 Issue

No issue.
Similar to #475, I thought Match could benefit from refactoring. Note that I will be cleaning it up further, getting rid of same_organization? and get_pet_id in the Authz PR too.

✍️ Description

This started with me changing the routes to the Rails convention. I personally found it a little confusing that sometimes we refer to adoptions as "adoptions" and sometimes as a "match". This PR tries to at least make it so that things using Match refer to it as a "match". This includes things like the strong params name.

The other portion of this PR works to get AdopterApplication logic out of MatchesController. I think this business logic belongs in the models. I also like setting withdraw for an individual object more as an instance method rather than as a class method like self.set_status_to_withdrawn was doing. Feels more intuitive.

Using update!

One thing I would like to bring attention to and get opinions on, is that I opted to use update! instead of update for these new methods. I was of two minds on it. The main difference is that update! is not silent and will raise an error. That seems undesirable in some cases.

However, where would this failure occur? Currently, retire_applications is called in AdopterApplication#create after a valid creation is performed. If something was going wrong where the applications could not update following a successful match, I think we would want to know about that. I also think the use cases here are fairly reliable and can't think of a case where I would expect them to fail.

Using mocks

I would also be curious of opinions on how I wrote the model tests too. I think mocking works well for some of those tests but would love to hear other's thoughts.

[nsiwnf]

Thanks for the cleanup on this! Taking a look now 👀

Yeah, at some point we thought "Match" made more sense than "Adoption" since we wanted to support "Foster" as well. "Match a pet with an adopter/foster"

[nsiwnf]

Running into a few errors locally - some are unrelated to this PR. Are you able to go through various flows on the application_review page?

[nsiwnf]

I think this should be match_params[:pet_id]?

Suggested change

	return params[:pet_id] if params[:pet_id]
	return match_params[:pet_id] if match_params[:pet_id]

[mononoken]

Thanks for the cleanup on this! Taking a look now 👀

Yeah, at some point we thought "Match" made more sense than "Adoption" since we wanted to support "Foster" as well. "Match a pet with an adopter/foster"

Thank you! That makes sense.

Running into a few errors locally - some are unrelated to this PR. Are you able to go through various flows on the application_review page?

Good catch. Your suggested change on line 46 fixes this I think. I have added that now. I just realized I was not getting this error because I had been running the server off of the authz branch instead of this branch and the authz branch already got rid of get_pet_id and same_organization?. Woops! Should be working now.

[nsiwnf]

lgtm!