closes #5 #11
closes #5 #11
Conversation
There was a problem hiding this comment.
Thanks @eldIsher ! Could you please take a look at these remarks? Your help is greatly appreciated! :)
| @@ -13,7 +13,8 @@ | |||
| 'prefix' => config('cookieconsent.url.prefix'), | |||
| ], function() { | |||
| Route::get('script', ScriptController::class) | |||
| ->name('script'); | |||
| ->name('script') | |||
| ->middleware('cache.headers:public;max_age=2628000;etag'); | |||
There was a problem hiding this comment.
Could you please explain why this line is necessary and how it is related to the issue you're solving? Thanks!
| use Illuminate\Http\Request; | ||
| use Whitecube\LaravelCookieConsent\CookiesManager; | ||
| use Whitecube\LaravelCookieConsent\Facades\Cookies; |
There was a problem hiding this comment.
We do not need the Cookies facade since the CookieManager is injected in the controller's __invoke method.
| class ResetController { | ||
|
|
||
| public function __invoke(Request $request, CookiesManager $cookies) { | ||
| $response = !$request->expectsJson() |
There was a problem hiding this comment.
Please do not use your own coding style rules within the package (I guess you're using an automatic CS fixer with different rules than ours). Thanks !
| ? redirect()->back() | ||
| : response()->json([ | ||
| 'status' => 'ok', | ||
| 'scripts' => $cookies->getNoticeScripts(true), | ||
| 'notice' => $cookies->getNoticeMarkup(), | ||
| ]); | ||
|
|
||
| $domain = config('cookieconsent.cookie.domain'); |
There was a problem hiding this comment.
| $domain = config('cookieconsent.cookie.domain'); | |
| $consent = config('cookieconsent.cookie.name'); | |
| $domain = config('cookieconsent.cookie.domain'); |
Since we're using $domain below, we could also extract the consent cookie name into a variable to make code more readable (see next suggestion).
| cookie: config('cookieconsent.cookie.name'), | ||
| domain: config('cookieconsent.cookie.domain'), |
There was a problem hiding this comment.
| cookie: config('cookieconsent.cookie.name'), | |
| domain: config('cookieconsent.cookie.domain'), | |
| cookie: $consent, | |
| domain: $domain, |
| // delete all defined cookies | ||
| foreach (Cookies::getCategories() as $category) { | ||
| foreach ($category->getCookies() as $cookie) { | ||
| Cookie::queue(Cookie::forget( | ||
| name: $cookie->name, | ||
| domain: $domain, | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
| // delete all defined cookies | |
| foreach (Cookies::getCategories() as $category) { | |
| foreach ($category->getCookies() as $cookie) { | |
| Cookie::queue(Cookie::forget( | |
| name: $cookie->name, | |
| domain: $domain, | |
| )); | |
| } | |
| } | |
| // delete all defined cookies | |
| $response = array_reduce($cookies->getCategories(), fn($response, $category) => array_reduce( | |
| $category->getCookies(), | |
| fn($response, $cookie) => $response->withoutCookie(cookie: $cookie->name, domain: $domain), | |
| $response | |
| ), $response); |
Did not test, but this way we do not need to use Cookie queuing and the Cookie facade.
| ### Middleware | ||
|
|
||
| Add `AddQueuedCookiesToResponse` to your `$middleware` or `$middlewareGroups`. The controller `Whitecube\LaravelCookieConsent\Http\Controllers\ResetController` uses cookie queue to reset cookies. | ||
|
|
There was a problem hiding this comment.
If the suggested code for removing cookies works, this is not needed anymore.
No description provided.