Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for string and DateTimeInterface as Cookie::$expire #19920

Merged
merged 6 commits into from
Aug 15, 2023

Conversation

rhertogh
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature? ✔️
Breaks BC? -
Fixed issues #19917

The \yii\httpclient\Response::parseCookie() function sets the Cookie::$expire as string.
This PR broadens the accepted types for Cookie::$expire from int to int|string|null.

@what-the-diff
Copy link

what-the-diff bot commented Jul 29, 2023

PR Summary

  • Enhancements in Cookie Expiry
    In Cookie.php, we've made improvements to the expire property. Now, it's flexible and accepts both timestamp and date formats, which lets you set your preferred expiration time format for cookies.

  • Improved 'Has' Method Handling
    A modification in CookieCollection.php enables the has method to understand and properly handle various forms of expire property. This results in more seamless cookie collection operations.

  • Response Handling for Different Expire Formats
    The sendCookies method in Response.php has been enhanced to handle various expire formats. This brings in versatility to our cookie handling.

  • Testing for Various Cookie Scenarios
    We've added a new test to ResponseTest.php. This tests different situations for cookies, making sure they work well in different configurations.
    This includes the sameSite element and the different ways the expire property can be set. This ensures our cookie functionality is robust and error-free.

@codecov
Copy link

codecov bot commented Jul 29, 2023

Codecov Report

Patch coverage: 68.18% and no project coverage change.

Comparison is base (8a21331) 48.89% compared to head (11016c0) 48.90%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19920   +/-   ##
=======================================
  Coverage   48.89%   48.90%           
=======================================
  Files         445      445           
  Lines       42786    42802   +16     
=======================================
+ Hits        20919    20931   +12     
- Misses      21867    21871    +4     
Files Changed Coverage Δ
framework/web/Cookie.php 0.00% <ø> (ø)
framework/web/CookieCollection.php 34.00% <58.33%> (+2.53%) ⬆️
framework/web/Response.php 75.79% <80.00%> (+0.22%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rhertogh rhertogh marked this pull request as ready for review July 29, 2023 18:07
@rhertogh rhertogh requested review from a team July 29, 2023 18:07
@bizley
Copy link
Member

bizley commented Jul 30, 2023

Ok, let's keep it as it seems that it's common thing to be prepared for other types than int. BUT since we are doing this let's allow DateTimeInterface as well.

framework/web/Cookie.php Outdated Show resolved Hide resolved
@rhertogh rhertogh requested a review from bizley August 4, 2023 14:09
…r PHP 5.4 and commited missing code for \DateTimeInterface processing in `\yii\web\Response::sendCookies()`
@rhertogh rhertogh changed the title Added support for string as Cookie::$expire Added support for string and DateTimeInterface as Cookie::$expire Aug 4, 2023
@bizley bizley merged commit 73902f0 into yiisoft:master Aug 15, 2023
50 checks passed
@bizley
Copy link
Member

bizley commented Aug 15, 2023

👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants