Jump to content

Recommended Posts

I am having a hard time figuring out a way to set the pathInfo within the request when testing. Here is the scenario: I am adding a global scope to a model in the boot() of that model that is conditional based on the route that given. This works great under normal usage but testing it fails the condition because the pathInfo within the request is only "/".

What I don't understand is that AFTER the response is returned from the $this->get() in the test, the request has all the correct data, but it is NOT populated beforehand at the route model binding level. It is populated by the time it hits the middleware, but that is too late to attach or not attach a global scope for a route model binding situation.

The page in question is protected behind auth middleware, so the $this->fakeUser(); is simply a quick function to be() a user;

Here is the model code:

public static function boot(){
  parent::boot();

  if(!request()->is('admin/*')){
  	static::addGlobalScope(new IsActiveScope());
  }
}

As you can see the code is just checking if the route is "admin/*", so any admin pages essentially. But not matter what I have tried, the route always comes through as "/" when it hits that piece of code and the route model binding level. If you do a dd(request()); AFTER the $response in the test, the route has been populated at that point and is correct. This is the test:

/** @test */
public function see_correct_active_icons_for_each_brand()
{
  $this->fakeUser();

  $brand1 = factory(Brand::class)->create(['name' => 'A', 'active' => true]);
  $brand2 = factory(Brand::class)->create(['name' => 'B', 'active' => false]);
  $brand3 = factory(Brand::class)->create(['name' => 'C', 'active' => true]);

  $response = $this->get(route('admin.brands.index'));

  $response->assertSeeInOrder([$brand1->displayStatus(),$brand2->displayStatus(), $brand3->displayStatus()]);
}

How can I fake this or is there another way to add the global scope in the manner that works like I have it now?  @requinix I see that you know a good amount about Laravel, any insight?

Link to comment
https://forums.phpfreaks.com/topic/308845-request-path-incorrect-during-testing/
Share on other sites

Managing the scope in your model based on request feels weird. I would have put the admin routes inside a group with middleware like

Route::group(["prefix" => "/admin", "middleware" => "with-inactive-models"], function() {

Models needing it add the scope, and with-inactive-models can disable it globally - at the simplest,

class IsActiveScope {

	private static $enabled = true;

	public static function setEnabled($enabled = true) {
		self::$enabled = $enabled;
	}

}

And IsActiveScope checks $enabled before applying itself.

The way I have it now works great in production, although I do understand it being weird to manage it the way I have.  The ONLY time it doesn't work is during unit testing.  I like your way of applying it but I don't think it will solve the original issue I posted about.  Reason being (primarily) is that when using route model binding, this would still fail to set I believe cause route mode binding appears to apply it's scopes at the route service provider level which is before the middleware level and hence would be too late to NOT apply the scope.

Hopefully that made sense.  Any other suggestions or maybe I'm wrong, but I feel I'm not after all the testing and messing around trying to make it work for several hours already.

1 minute ago, fastsol said:

I like your way of applying it but I don't think it will solve the original issue I posted about.

I think it will. While I'm not sure why detecting admin/* isn't working, that process uses the request URI while mine uses the router. They're different mechanisms, and as long as the test goes through the router (which I can't imagine it would not) then the router should apply the middleware appropriately.

Although one point worth mentioning is that the middleware should undo itself after the request, so

IsActiveScope::setEnabled(false);
try {
	return $next($request);
} finally {
	IsActiveScope::setEnabled();
}
1 minute ago, fastsol said:

Reason being (primarily) is that when using route model binding, this would still fail to set I believe cause route mode binding appears to apply it's scopes at the route service provider level which is before the middleware level and hence would be too late to NOT apply the scope.

Routing is one of the first things that happens. It's before the controller. Which should therefore be before most models (except maybe your user) are running queries.

The scope is always added to the list of global scopes. What varies is whether the scope actually affects the query - like adding its ->where or whatever just before the query is executed - and that's what is controlled by $enabled.

I've been testing the last 30 minutes by making 2 middleware that uses session() to set the decision to add the scope.  Session seems to be the only way to retain the decision throughout the application due to varying class scopes (not the model scope) and such.  So far it's working in production and the quick unit tests I ran.  I have to do some more testing to ensure it's working as I need, but so far it looks good.  Thank you very much, I just needed an idea of how else to set the decision besides the request() and you helped me there.

Ok so I was wrong, my session method was not working either.  I'm starting to see the issue here though.  When I run the site normally the middleware is triggered BEFORE the route model binding, but when unit testing the middleware is triggered AFTER the route model binding.  I have confirmed this by die dumping in multiple places in the chain to see which gets triggered first.  The results are as I stated above, unit testing the model binding is first in the chain which is why no matter what I do here it will likely fail cause I am not able to set any variable/class/anything beforehand to tell the model not to use the scope.

Like I said originally, this is only an issue during testing, and really only makes it so I can't test 1 thing on certain models on either the customer side or admin side depending on when the scope is being applied.  So maybe I just give up on this and know that that 1 specific test can't be handled through unit testing.  I may end up doing Dusk anyway, so it would be tested then correctly.

It appears that I can set a static method within the unit test itself that does transfer over when doing $this->get(route(''blah));.  So I should be able to do the middleware method as you suggested, but then manually set the $enabled to false within the test before using $this-get().  This is a workaround I guess, but it still feels icky since I having to go around the actual code that runs the site normally to make it work just for the test.  Maybe it's a compromise I'll have to live with.

I had previously tried your last suggestion with the env settings and that made it work in reverse where now the customer side of the code was failing and the admin side was passing because I still couldn't set a true of false value based on a condition, the env value was just set as soon as the test started.

I know I'm no where near a "great" coder, especially with Laravel as this is only my second site using it, so many of these little quirks about testing and env are slightly new to me.

50 minutes ago, fastsol said:

I had previously tried your last suggestion with the env settings and that made it work in reverse where now the customer side of the code was failing and the admin side was passing because I still couldn't set a true of false value based on a condition, the env value was just set as soon as the test started.

Yeah, I got a little confused about when you wanted the scope. It's about the admin routes, not about whether it's doing testing.

Having stepped away from the computer for a couple hours,

public static function boot(){
  parent::boot();

  if(!request()->is('admin/*')){
  	static::addGlobalScope(new IsActiveScope());
  }
}

The model will only be booted once, and it'll probably happen early. Like for the very first request. Which doesn't help with testing - unless you're requesting against the web server, but it doesn't sound like it.

The admin/* check would have to be inside IsActiveScope when it's being applied. So basically what I said before, except instead of the middleware and flag (which I still recommend) you're testing the request() path.

  • Thanks 1
1 hour ago, requinix said:

The admin/* check would have to be inside IsActiveScope when it's being applied.

This is the piece of information that made it work how I want in all environments.  I now understand that in the boot() it was just adding the scope to the list but not actually running it at that moment.  By putting the admin/* in the apply() of the scope itself the request()->path() had been populated and worked as expected in testing also.  Thank you so very much, I've been beating my head for a while on this as I kept coming back to it when I'd make a new test that it was affecting.

This is what I ended up with:

class IsActiveScope implements Scope
{
	/**
	 * @param Builder $builder
	 * @param Model $model
	 * @return Builder|void
	 */
	public function apply(Builder $builder, Model $model)
	{
		if(!request()->is('admin/*')) {
			return $builder->where($model->getTable() . '.active', '=', 1);
		}

		return $builder;
	}
}

And just this in the boot of the model:

public static function boot(){
  parent::boot();

  static::addGlobalScope(new IsActiveScope());
}

 

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.