scenes. So as an example, maybe from the console, they could submit a POST request, basically bypassing the main create form entirely. And it looks like, according to this, if they send through an archivedChannelID, that will still work. Not a huge issue, but nonetheless, probably something we should protect against. So let's see what they've done here. So on ThreadsController, when you store a new thread, we're just going to say the channelID. Okay, this is brand new. So the channelID, yes, is required. And yes, it needs to exist on the channels table. So you can't give us an ID of 90 if there aren't 90 channels. But we've also added this where clause. It also needs to not be archived. So if archived is set to true, then that, of course, is going to fail. It looks like we have a comment here. Personal preference, but since all the other rules are defined as a string, the existing rule could be rewritten as this. Yeah, that's true. It's good

It looks like we have a comment here. Personal preference, but since all the other rules are defined as a string, the existing rule could be rewritten as this. Yeah, that's true. It's good for consistency, but I'll tell you this. I run a website dedicated to teaching Laravel, among other things. And when I see this, especially this section right here, even after all these years, I can't remember what each one refers to. It's too hard. I have to look it up almost every single time. So in other words, true. What does true mean? I couldn't tell you right now. But if I look at this, okay, well, we have a new validation rule where we want to say the ID column on the channels table has to exist. So what you pass through has to exist. And further, the archive_status has to be false. That is so much more easy for me to read versus this. So we'll skip that. Okay, next we have a test here. A new thread cannot be created in an archive channel. All right,

has to be false. That is so much more easy for me to read versus this. So we'll skip that. Okay, next we have a test here. A new Thread cannot be created in an archive Channel. All right, so this is an important thing. When you submit a pull request that fixes some kind of bug, and you could call this a bug in this case, in certain situations, the User can insert data or assign a Channel that you don't want to allow. So in these situations, it's very helpful to have a test that confirms the bug itself. Otherwise, I'm just taking your word for it that it doesn't work. Here, though, if you give me a test, I know for sure, oh, here's an example of the functionality we want, and the submitted code makes that pass. It's very useful. So let's go over it together real quick. Given we have a Channel and we publish a Thread, well, once we redirect to wherever that endpoint sends us, we're going to expect to see the title and the body. But

over it together real quick. Given we have a Channel and we publish a Thread, well, once we redirect to wherever that endpoint sends us, we're going to expect to see the title and the body. But then if we archive the Channel and try to publish a new Thread using that Channel, we expect there to be errors within the session. And it looks like we have another comment. We dropped lines 162 to 168. So he's saying right here, the other test pretty well covers this case already. And yeah, that could be fair. We'll take a look at that. Okay, so this looks fairly good to me. I'm going to go ahead and merge that into the code base. And then very quickly, I'm going to pull it in, run our phpunit suite, and that seems to be looking good. Let's go to that createThreads test real quick. Okay, so here's the new one. A new Thread cannot be created in an archived Channel. So you'll see that's passing. However, if we go to ThreadsController and we bring it to what

test real quick. Okay, so here's the new one. A new thread cannot be created in an archived channel. So you'll see that's passing. However, if we go to ThreadsController and we bring it to what we had before, so I think before we had something like exists on the channels table, specifically the ID column. That's basically what we had before. So if we gave that a run, yeah, now that test reproduces the issue we had here. Failed asserting that false is true. So we confirm that once again, we'll create a channel, publish a thread, and that all seems to work. Now, I do happen to agree in this particular case with the user Indemity, whatever his username was. We already have this logic in another test. So maybe we can get rid of it entirely. But it should still fail, and it does. Given we have a channel, and we archive that channel, if we try to publish a thread using that archived channel ID, there needs to be an error in the session. And we might even

fail, and it does. Given we have a channel, and we archive that channel, if we try to publish a thread using that archived channel ID, there needs to be an error in the session. And we might even want to add an additional test that confirms under no circumstances is there a thread created. So that would be like a second layer and just kind of assurance. Yes, there needs to be errors in the session, but it's possible you could add errors to the session and still save it by accident. So that's like a second layer, to be sure. Anyways, if we bring this back to what we had before, we run it now, that's passing. And then finally, yeah, if we wanted to say this assertEquals maybe zero, and then we could say appThreadCount. Give that a run. And now that does return green. Next, if we want, we could make the archive process part of the instantiation. Like, if you wanted to do that, that would be fine. I have no preference either way. It's nice

does return green. Next, if we want, we could make the archive process part of the instantiation. Like, if you wanted to do that, that would be fine. I have no preference either way. It's nice using the API, or it's fine and simple here if you want. You could even add a model factory state if you want. It just depends if it's necessary. Why don't we keep it like that, though? Create a Channel, try to publish a Thread to that Channel, and assert we have errors in the session, and also assert that we have not created any Threads. Or you could even say, hmm, this assert, can we use count, assert count zero, and then say channel.threads. So make sure, yeah, so make sure, let's run that, that screen. Yeah, so if you try to get any Threads assigned to that archive Channel, absolutely, that should have a count of zero. Alright, so let's go up to GitHub. Now we can see here I removed a comma. Generally, when you make a commit, try to make sure only the things

but the maintainer hasn't looked at that code in potentially months, that piece of the code in months. So just break it down and explain exactly what the problem is and what you've done to solve it. Anyways, in this case, it does look like it's a simple issue, so no big deal. So the option value, yes, what we had here is we're setting the selected state, the selected option here. And in this case, it's just handling the case that validation fails. So we will grab the old value for archived or just default. So I think that's perfectly fine, and I will squash and merge that in. All right, let's keep going. How about this? Change the look of a new reply section when viewing a thread. So if this is UI, okay, this is from Wilbur. Currently, it looks very flat. This is a temporary fix while the design stuff comes along. I also made the WYSIWYG editor a bit bigger. Okay, so I guess in this case, he just added a white background. Yeah, I think before

purposes. Using the danger class on the table row is, in my opinion, slightly more semantic and serves a better indicator. So yes, in this case, it might be more semantic. However, it's going to change the color. I know maybe that should have nothing to do with the semantics, but in a practical sense, at the moment, this is going to refer to red. So why don't we just see what it looks like. So I'm signed in as the administrator. We'll go to channels here. Yeah, at the moment, it's showing up as blue, which I would even agree is not a great color. You want more of a muted grayish color. But yeah, if we were to change that to danger, actually in this case, it's a muted red. That's not too bad. I was expecting more of like an alert red. So you know what? I think that's fine. Let's go ahead and pull that in. All right, what else? This one from Tychus. Use with count to optimize queries. All right, let me take a quick look. So on the administrator channels controller,

fine. Let's go ahead and pull that in. All right, what else? This one from Tychus. Use withCount to optimize queries. All right, let me take a quick look. So on the AdministratorChannelsController, we fetch all of the channels, and it looks like they're adding the count, so the number of threads as part of that query. And you can see here, yep, so let's go to channels.index. Let's take a look at this. And that's line 27. Yeah, okay, so I should have caught this. Whenever you're within a loop, and you make what translates to a SQL query, right? So in this case, we're saying channel, get your threads relationship, and fetch me the number of records. So that is performing an SQL query. And generally, that's not a big deal, except when it's in a loop. That's when you end up with these n plus one problems, because it's not just one query anymore. It's one query for every single item in the loop. And if you have potentially 500 items in the loop, then that's 500

end up with these n plus one problems, because it's not just one query anymore. It's one query for every single item in the loop. And if you have potentially 500 items in the loop, then that's 500 needless queries you're going to make. So what this PR does is it adds the thread count as part of the query. So it will do maybe one, maybe two additional queries total, versus n plus one, versus potentially 500 queries. And then it updates that to use the new property that is returned. So that's good. We'll say, nice catch, and merge it in. All right, what else we got? Maybe one or two more, and then we'll call it a day. Here's another one from Tykus. The thread is subscribed to access our property is not used by our Vue app kernel. Is it not? Maybe at one point it was, but maybe we removed it. This is one thing with maintaining a project that can often be difficult, is you will write code, and then maybe at a later date, a year from now, two

but maybe we removed it. This is one thing with maintaining a project that can often be difficult, is you will write code, and then maybe at a later date, a year from now, two years from now, you will remove the part of your project that references that code. So then, if you're not careful, you end up in situations where that original code that's not being used still exists. And this can be a tricky thing. Often you have to take time to really scour your code base and actually check, is there any point that we're triggering this method or we're calling this API? If not, can we safely get rid of it entirely? These are all things you want to think about. So it looks like we may have a case of that. So a Thread is subscribed to access our property is not used. So let's take a look at that. I go to thread.php. So we have, where is it? Right here. So we added this section to our appends property. And this means,

not used. So let's take a look at that. I go to thread.php. So we have, where is it? Right here. So we added this section to our appends property. And this means, if you need a recap, if we were to cast this Thread to JSON, to an array, well, as part of that, we want to append this custom accessor isSubscribedTo. And in this case, it's just adding the results of this call here. Figure out if the User is currently subscribed to the Thread. Now, according to this PR, we're not making use of that anywhere. So all he has done is removed that. So let's see, are we really not making use of that? Let's just look for isSubscribedTo. All right, so in our thread page, actually, we are making use of that. So that warrants its existence as an accessor. But yet what he's saying here is, well, look, you're making it available in the array form. And at the moment, we're not using it.

Does it really? Yeah, because if we have a global scope, let's see how this works. So if we go to Thread, and actually real quick, let's pull in all of our PRs. Okay, there we go. Anyways, we're back to our Thread class. And I'm sorry, let's go to Channel. Yeah, so you'll remember, right in our boot method, we added this global scope. So for any Channels we return, the archive status has to be set to false. So it sounds like if we need to determine the path for a Channel that is archived, this is going to end up being an issue. Because if we get the relationship, that will return null. Let me try an example here. Let's boot up php artisan tinker. And let's see, let's create a Thread. And we'll save it to T. Okay, if we fetch the Channel, there we go. But why don't we say channel, and we're going to say archive. Okay, now, if we fetch the Channel,

let's create a Thread. And we'll save it to T. Okay, if we fetch the channel, there we go. But why don't we say channel, and we're going to say archive. Okay, now, if we fetch the channel, you can see that archived has been set to true. Which means if I create a path, well, yeah, that's the original value. But I do have to get a fresh copy of the thread, and then generate a path. And there we go. There's the error that's been brought up in the PR, trying to get a property of a non object. Because again, if we fetch the channel, that is now returning null. So it looks like here, if we fetch the channel relationship, this makes sure that we say no global scope. And then we have a test that reproduces it. Archived channels should not influence the existing thread. So here, the verification, the proof that this bug exists, is we sign in the administrator, we create a channel, we create a thread, and we assign that

influence the existing thread. So here, the verification, the proof that this bug exists, is we sign in the Administrator, we create a Channel, we create a Thread, and we assign that channel name, and then we get the path. That path should be the full path to the Channel, regardless of whether it's archived or not. Then he archives it. And the assertion is that, well, yes, if we get a new path, that should still equal the old one, there shouldn't be an error. Okay, so I think this is fine. Let's go ahead and squash and merge that in. However, real quick, let me pull in those changes. And go back to code, and we'll go to ChannelAdministrationTest. Okay, and where is it? Okay, so here's the test, and that is in fact returning green. Let's just do a little formatting. For things like that, that's its own operation, so I like a line break. Next, we have a Channel, we create a Thread. Now, is there any reason to create the Channel manually?