Here, I have a controller for interacting with Member on a Team. If we take a look at the methods, we have two core actions for adding and removing the Member from the Team, and then four helper methods. So together, I'd like us to go through this file and clean it up. I think there's some good, real-world educational material here. Okay, so let's get started with the store method. This is, again, the endpoint for adding a person to a Team. Now we begin by validating the request. Then we invite the email address to the Team. But then you'll see here, and then this stuff.

a bit of JSON. Otherwise, we can do a redirect response. And there's nothing wrong with that at all. But just make sure that you do need to provide both responses. So it might turn out that you're only ever going to hit this endpoint as an AJAX request, in which case you'll never need to do a traditional redirect response. But in other cases, you'll want to respond to both types. So it just sort of depends. Now, in my case, I happen to know all of the requests coming in will be AJAX requests. So that means I really don't need to do the check anymore.

and that's fine. Already, though, this looks a good bit cleaner. Let's drill down, though. How do we invite a User to the team? So if we take a look at this, I do see that we try to find a User. But if we could not find a User, it sounds like it's somebody who doesn't have a Laracast account, in which case we should send them an invitation, an invitation to sign up and join the team. So you'll see that's what we're doing here. If you don't have a User in the system, then get our Team model and call an invite method.

So you'll see that's what we're doing here. If you don't have a User in the system, then get our Team model and call an invite method and then return a response. Otherwise, we have an existing User, so we can add them immediately to the Team and then once again return a response. Okay. So this is an example where I feel like we can clean things up a good bit. So for one, does the controller really care how the User is invited and added to the Team? Not really. I mean, I just kind of want to say, okay, I have this email address, invite them to

Not really. I mean, I just kind of want to say, okay, I have this email address, invite them to the team. Then the model can decide in what form that takes. Do we first need to send out an invitation or can we just go ahead and add them immediately? The model can be responsible for that. So with that in mind, we kind of just want to do something like that. Give me the team and then call invite and pass through the user. So now this should be the place where we decide whether we do that or that. Okay.

So now this should be the place where we decide whether we do that or that. Okay. So immediately, if I run the code, everything's going to blow up as we would expect. All right. So let's drill down to this invite method. You'll see before it was expecting an email address, but now we're giving it either a User instance or null, and I don't like that. So we'll talk about that in a moment. But first let's drill down to invite and you'll see invite here generates a brand new invitation for the team and then it fires it off to the User.

But first let's drill down to Invite and you'll see invite here generates a brand new invitation for the team and then it fires it off to the User. But now we've decided we're actually inviting a User. So we're getting away from the primitive email string and we want to switch over to a User, whether it's a guest User or an existing User. So we'll update this. And now we can say generate a new invitation and fire it off to the email address of our User. Okay. But now we have to handle that situation where we already have an active User.

Okay. But now we have to handle that situation where we already have an active LayerCast User, in which case they don't need to receive an invitation link. So we could say, well, if this User exists in our system, so we can use that exists property that will determine basically, do they have a primary key? Has it been persisted? If so, well, then I can just call add on my team. So notice this code was originally in the controller, but now we've dropped down a level and we've let the model be responsible for it. All right.

and we've let the model be responsible for it. All right. If I run my tests again, it's still going to fail. So we're calling the invite method. We're expecting a User, but null is being received. Okay. Let's think about why. Well, we find the User and then we call invite and we send it through. But yeah, in certain situations, null is going to be returned. Like if we are inviting a person that has never had a Laracasts account.

But yeah, in certain situations, null is going to be returned. Like if we are inviting a person that has never had a Laracasts account. So here's what we could do. Your first thought might be, well, if you don't have a member, we do want to send through a User. So I could say member equals a brand new User with that given email. And if I run my code, we're back to green. Great. But there's another way that we could clean this up a good bit. We could say User::first() or new User().

But there's another way that we could clean this up a good bit. We could say user first or new. This is effectively what we want. Give me the first User in the database that matches these attributes or create a brand new one. Don't persist it. So it's not first or create, it's first or new. So then I could say where the email address matches. So that would allow me to delete this code entirely. Okay.

So that would allow me to delete this code entirely. Okay. So now if I run the code again, it's still returning green. Okay. So next I can get rid of all of this. And then we could say if the member does not exist, then return this. But yeah, so we need to decide how we want to deal with this because I don't like that I'm returning two different JSON blocks here. Now as a general rule, I don't have an issue with multiple return points for a method, but there is truth that the more you add, the more complicated the code gets.

Now as a general rule, I don't have an issue with multiple return points for a method, but there is truth that the more you add, the more complicated the code gets. And then second, if I can, I'd rather merge this down to a single JSON response. So you'll notice it's mostly the same. Both of them have an email. Both of them have a status, but the status is different for each. And then both of them had this odd, I'd even say bizarre Gravatar URL. This is probably something from five years ago that needed to be added, and I'm not exactly sure why. So in a perfect world, we would get rid of that entirely.

So let's see how we would merge these. Well, let's just imagine we only did it once. Here, ID, well, in one case, it would be null, and in another case, it would be the ID of the User. I think that's okay. Next, the email is the same. The status will need to switch. So we could say if the member exists, then the status is active, otherwise it's invited. Finally, the Gravatar is the same. Okay.

But yeah, this is much cleaner and much easier to take in than what we had at the beginning of the lesson. We went from, I don't know, 15 lines, maybe more, down to just a handful. And most importantly, it's easy to understand. Get the team, invite a User, and then return a Response. Okay. So let's go back up. Our store method is looking pretty good. Our invite method is looking pretty good. Next, let's see.

Our invite method is looking pretty good. Next, let's see. So we had this refresh method. Are we calling it anymore? Ah, we're not. Okay. So you'll remember earlier in the video when we decided that we're only ever responding to AJAX requests, we got rid of any calls to the refresh method, which means I can delete this entirely if I want. But a quick note, refresh might be something that's useful in your system if you want an

this entirely if I want. But a quick note, refresh might be something that's useful in your system if you want an easy way to call a method that will flash something to the screen and then perform a redirect refresh. So if you do find yourself reaching for something like that, just pull it up to your parent Controller. And that way, any Controller can call that method. Because if you think about it, it's nothing specific to working with teams. It's a simple Controller helper method. But yeah, in our case, we're not using it.

It's a simple controller helper method. But yeah, in our case, we're not using it. So I'm going to remove it entirely. And now our class is looking like this. It's getting better. What else? Let's go to this destroy method. And immediately to my eyes, this feels a little sloppy. Even if you're a newcomer, I hope you can see that. It feels a little off.

Even if you're a newcomer, I hope you can see that. It feels a little off. We have one, two, three different returns. In some cases, we're checking to see if we need a JSON response. In other cases, we're not. This is the sort of code you can end up with sometimes after years of maintenance and contributions. So I want to see if I can fix this. And the first step is, we've already decided we're only responding to AJAX requests, and we happen to know that for sure. So with that in mind, I no longer need to check if the request wants JSON, because I

we happen to know that for sure. So with that in mind, I no longer need to check if the request wants JSON, because I know it does. That means I can remove the flash message and the redirect. Next we can see... I'm going to make this a little easier for your eyes. So I'm going to move that out. Okay. Let's go through it. We're trying to find a User by an ID.

Let's go through it. We're trying to find a User by an ID. But as we'll see, this is a little more shady than it looks. But try to find a User, and if you didn't find them, then we are canceling the invitation. Yeah, so this is a similar thing. So imagine we invite a User to a Team, but before the User has signed up, you reverse that invitation. You cancel it. That's what we're doing here. If we couldn't find a User, then they haven't yet signed up.

That's what we're doing here. If we couldn't find a User, then they haven't yet signed up. So all we have to do is track down the invitation that we sent to the User and cancel it. But otherwise, if we did find a User, then we will call removeMember on our team. That method simply removes them from the team and then figures out how to deactivate them. Now this is another case where I think we can merge things a good bit. So why don't we reverse this? Let's say if we found a User, then removeMember and return the status. Otherwise, just bear with me for a moment to make this clear what's happening. If we found a User, removeMember and provide adjacent response.

Otherwise, just bear with me for a moment to make this clear what's happening. If we found a User, remove the member and provide adjacent response. Otherwise, cancel the invitation and return adjacent response. Well to start, I would like to return a single response. And then, like before, we could do this here. We could say, does the User exist? If so, member removed from team. Otherwise, invitation canceled. Now I can remove this and this. So we've normalized it.

Now for my domain, I think of them as team members. So let's be consistent. Find the member. If we found one, remove them from the team. Otherwise, cancel the invitation and then ultimately return a response. Okay, better. Let's run the tests. Ah, and it does fail. Yeah, this will be the same thing where we have to normalize. So when we find a User by their ID, we either get a User instance or null, right?

Yeah, this will be the same thing where we have to normalize. So when we find a User by their ID, we either get a User instance or null, right? So we're trying to see does null exist, and that's why it's failing. Okay. I think we need to dig down to find User by ID and clean this up, because I think this is a big mess. So find User by ID, we accept an ID, and then the team, and then we try to track it down. Look for the User in the database that has that ID, but who also is part of that team. Now here's where it gets weird. If we could not find the User, then do another check, but this time see if we can find an

It just seems very strange to me. But nonetheless, these are the sorts of things you will often have to deal with, especially in situations where maybe a different person or a different team manages the JavaScript and you manage the server side. You won't always have the ability to change things up like this if they are depending upon it for an AJAX request on the front end. So with that in mind, if we have to stick with, okay, hunt down a User with a given ID or email, at the very least I want to clean this up, because findUserByID is not correct. It's findUserByID or email, or we could just clean it up to findUser. Next, it could be the ID or it could be the email.

It's find User by ID or email, or we could just clean it up to find User. Next, it could be the ID or it could be the email. So let's make it more generic, key. Next up, we're trying to find a User with this ID. Is there any reason to also scope it to the team? Not really. At least to my eyes, I don't see any situation where that would change anything. So maybe we could remove this entirely. So if we took that approach, I no longer need the team. So I could clean that up and let's find the usage here.

So if we took that approach, I no longer need the team. So I could clean that up and let's find the usage here. Yeah, find the User. So I no longer have to pass that through. All right, how are we doing here? Okay, so we're still getting property exists of non-object, which means find User in some scenarios is still going to return null. So let's keep working. We could then say $user equals User::first() and then, and don't worry, we're going to clean this up, but then if we still couldn't find a User, if that was equal to null, then at

We could then say $user equals User::first() and then, and don't worry, we're going to clean this up, but then if we still couldn't find a User, if that was equal to null, then at the very least, I want to normalize things. So I will create a fresh instance of User like this. Otherwise, we will return the $user. Okay, now I assure you, this looks rough to my eyes too, but let's get it to green and then we can incrementally refactor. So I'm going to run it and you know what? I would have expected that to return green. It disinvites a member from a team.

Okay, so now we can tweak this in a good bit, in fact. Let's break it down. What are you trying to say here? Find a User with a given ID or with the given email and give me the first result. And if you didn't get anything, then create a brand new User. Okay, how about this? Find the User by that ID or where the email matches. And we can do that. So now, run the tests. We're still at green, but way cleaner.

So now, run the tests. We're still at green, but way cleaner. Next we have that same thing again where I'm calling it user. We want to normalize it. We want it to be member. Okay, two return types, no problem there, but if you'd rather do this, that would be fine as well. Or you could say, return this member or a new User, like that, and then clean it up. Run it. Still at green.

Run it. Still at green. All right, way better than what we had before. Still kind of wonky that we're trying to find a User by ID or email, but that's the way it goes sometimes. So let's come back and take a look at our code. store method is good. destroy method is better, but I still think it needs a little bit of work here. State is pretty clean. Pretty clean.

State is pretty clean. Pretty clean. Pretty clean. Okay, so the only remaining step is my destroy method. But there is one thing I want to be careful of. So what I see here is either way, a Person is being removed from a Team. The only difference is in one form, the person's invitation is being removed, but they're still being removed. So I would like Team to be in charge of this rather than the controller. We're going to drop down a level and let the Team model decide in what form it removes.

So I would like team to be in charge of this rather than the controller. We're going to drop down a level and let the Team model decide in what form it removes the member. So with that in mind, what if we just said, team, remove member? All right, well, let's run the test. Everything blows up. And further, if I drop down, I'm now working with my Team model itself. So you'll see, I want to show you this. If I look for my team tests, I actually have three different ones. I have a controller test, a general feature test, and then a model test.

If I look for my team tests, I actually have three different ones. I have a controller test, a general feature test, and then a model test. So if you ever want to test multiple methods from different classes, of course, you can either trigger the entire suite or you can tag them like this. Let's go to this first one. And at the very top, I'm going to give it a group name of teams. Okay, next, let's go to the second one. And then finally, the main one we've been working off of. Okay, so at this point, I can now say phpunit, but only trigger the tests in the teams group.

Okay, so at this point, I can now say phpunit, but only trigger the tests in the teams group. Okay, and this is what we want. So now some things are failing as we would expect. So here's what we'll do. Let's come back to TeamsController. I want to make sure we're fully at green here. So let's run it again. And no, we're not. Okay, so I never want to begin a new refactor if my current tests are not returning green.

And no, we're not. Okay, so I never want to begin a new refactor if my current tests are not returning green. It gets too tricky that way. So let's patch up this issue, and then we'll move on. So it remembers the total users and invited guests. Okay, so I already know what this is. If we run it, we create a team with two members, and then we try to invite a person to the team. And if we check the team count, that should be three because we include the members and the invited users.

And if we check the team count, that should be three because we include the members and the invited users. So if I go to this inviteToTeam, you'll see that we're sending through an email when as we as we did together, we actually are now expecting a User instance. So now if I run it again, okay, one more issue, and it's probably the exact same thing. It can invite a person to a team. So let's run that and same thing, new User. Okay, run it one more time. We're back to green. Okay, so now I feel confident starting a new refactor.

We're back to green. Okay, so now I feel confident starting a new refactor. I'll go back to my controller here, and this is what we want to end up with. So if I run the tests, it's all going to blow up, right? So let's begin. Team, removeMember. So you don't need to worry about this stuff too much. We're just taking them off the team and then deciding how to deactivate them. But I will say right here, so basically, if the member is not currently on the team, then simply cancel the invitation, kind of writing it out for myself to start.

But I will say right here, so basically, if the member is not currently on the team, then simply cancel the invitation, kind of writing it out for myself to start. So what I could say is, I have a relationship between teams and invitations. So I could say, if the invitations for this team has one where the, I call it recipient, it's basically the email address. So if we have an invitation for the given user, and I'm sorry, once again, gosh, got to call it member. If that's the case, then we need to cancel the invitation. Now here's a cool thing. Here's what we had before.

Now here's a cool thing. Here's what we had before. So I'll paste that in. However, we no longer have to do this. So take a look. We're trying to track down an Invitation record that was sent to the given User, effectively, and then we call cancel. But if we remove it, we've already tracked down the Invitation. So I could save that, and that will give me an Invitation instance, and that's an Eloquent model.

So I could save that, and that will give me an Invitation instance, and that's an Eloquent model. So there, I can just say, okay, cancel it, and then finally return. Okay, so now, if I run my tests, with any luck, nope, no luck. It fails. Let's see. Oh, yeah, collection cancel does not exist. So I forgot to say, let's see, this invitation's where the recipient, yeah, that's going to return to me a collection of all users with that email. But we already know there's going to be one, so you could do this, like this, but you could

return to me a collection of all Users with that email. But we already know there's going to be one, so you could do this, like this, but you could also just say firstWhere, and that will effectively do the same thing. Okay, so let's give that a run. Okay, so the error's gone, but we now have a new failed test. It removes a member from a Team. All right, so there must be an issue. Let's go to TeamMembersController, invite, get the team, invite the user. If the user exists, oh, there it is. I can already tell that's it.

If the user exists, oh, there it is. I can already tell that's it. So we are saying if the user already exists, then add them, but then we just generate a new invitation. So that's not what we want. We need to return. But now I can see this method could either return a user or an invitation, which I don't love. I'd rather it be one thing. So let's return the user who was invited.

I'd rather it be one thing. So let's return the User who was invited. So we'd keep that, and then return the User. Okay, that might have broken something, and no, it's working, excellent. So I guess I never needed to test that result. It wasn't something I used in the code. So now, if we go back to removeMember, this method can now be responsible for checking. Okay, do we need to cancel an invitation, or should we do something different entirely? So if we jump back up to our controller, let's go to, where is it? Right here.

So if we jump back up to our controller, let's go to, where is it? Right here. Yeah, now the controller doesn't have to be responsible for this. So I can remove that, which means team can be inlined, and member can be inlined, and we get that. Last thing, findUser, I'm still not sure if that's the right name. We are finding the user who matches this key, or returning a new one. So why don't we change it to findUser or new. I just want to be specific here. And this is what we get.