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 "ValuesOnly" property to Cookie and QueryString layout renderers + support for multivalue cookie keys in ASP.NET core #307

Merged
merged 2 commits into from
Aug 1, 2018

Conversation

alexangas
Copy link
Contributor

Fixes #294 by adding a ValuesOnly property to AspNetLayoutMultiValueRendererBase and implements it for sub-classes AspNetRequestCookieLayoutRenderer and AspNetRequestQueryStringLayoutRenderer. This also includes a fix for multi-value cookie unit tests that slipped through my previous PR, and simplifies Core vs Full framework implementation and unit tests.

Also fixes unit tests not functioning correctly for full framework AssemblyVersionLayoutRender due to incorrect project references.

…derer, AspNetRequestCookieLayoutRenderer - adds ValuesOnly property and tests, plus fixes multi-value test bugs in the latter
@codecov
Copy link

codecov bot commented Jul 29, 2018

Codecov Report

Merging #307 into master will increase coverage by 1%.
The diff coverage is 97%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #307   +/-   ##
=====================================
+ Coverage      59%    59%   +1%     
=====================================
  Files          31     31           
  Lines         396    401    +5     
  Branches       88     91    +3     
=====================================
+ Hits          232    237    +5     
  Misses        129    129           
  Partials       35     35
Impacted Files Coverage Δ
...enderers/AspNetRequestQueryStringLayoutRenderer.cs 74% <100%> (ø) ⬆️
...outRenderers/AspNetLayoutMultiValueRendererBase.cs 98% <100%> (ø) ⬆️
...youtRenderers/AspNetRequestCookieLayoutRenderer.cs 85% <94%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcbd271...1c12268. Read the comment docs.

@304NotModified 304NotModified self-requested a review July 31, 2018 13:29
@304NotModified
Copy link
Member

Thanks!

@304NotModified 304NotModified self-assigned this Jul 31, 2018
@alexangas
Copy link
Contributor Author

@304NotModified By the way, happy to update wiki for this and also my assembly version work.

@304NotModified 304NotModified changed the title Adds ValuesOnly property to Cookie and QueryString layout renderers Adds ValuesOnly property to Cookie and QueryString layout renderers + support for multivalue cookie keys in ASP.NET core Aug 1, 2018
Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work! Thanks! Also for the unit tests :)

@304NotModified 304NotModified changed the title Adds ValuesOnly property to Cookie and QueryString layout renderers + support for multivalue cookie keys in ASP.NET core Added "ValuesOnly" property to Cookie and QueryString layout renderers + support for multivalue cookie keys in ASP.NET core Aug 1, 2018
@304NotModified
Copy link
Member

@304NotModified By the way, happy to update wiki for this and also my assembly version work.

Please do, you could add: added in NLog.Web / NLog.Web.AspNetCore 4.6 :)

@304NotModified 304NotModified merged commit 233e160 into NLog:master Aug 1, 2018
@alexangas alexangas deleted the cookie-and-query-valuesonly branch August 2, 2018 16:26
@304NotModified
Copy link
Member

Looks good! Thanks!

We will release this after NLog 4.5.8 is released :)

@snakefoot snakefoot added ASP.NET Core ASP.NET Core - all versions and removed ASP.NET Core 1 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASP.NET Core ASP.NET Core - all versions ASP.NET 4 ASP.NET MVC Classic enhancement feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aspnet-request-cookie Value Only
3 participants