[PM-38329] policies stop converting policy data on every response - #8204
Merged
JaredScar merged 11 commits intoAug 24, 2026
Merged
Conversation
…JsonConverter for Data property, enhancing null safety and JSON handling. Introduce RawJsonConverter utility for efficient raw JSON serialization/deserialization. Update related namespaces and remove unnecessary deserialization logic.
…ertions, improving JSON handling and readability. Introduce new tests for PolicyResponseModel and PolicyStatusResponseModel to ensure consistent serialization with legacy implementations. Add RawJsonConverterTests to validate raw JSON serialization/deserialization behavior.
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the switch of Code Review DetailsNo findings. The two open threads already have author responses explaining the intentional |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
JaredScar
added
t:tech-debt
…policies with no stored data by serializing as "{}" instead of null. Update PolicyResponseModel to allow setting Data property. Introduce RawJsonSchemaFilter to adjust OpenAPI schema generation for properties using RawJsonConverter. Add tests for PolicyStatusResponseModel and RawJsonSchemaFilter to ensure correct serialization and schema behavior.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8204 +/- ##
==========================================
- Coverage 68.57% 63.31% -5.27%
==========================================
Files 2401 2403 +2
Lines 104043 104069 +26
Branches 9426 9432 +6
==========================================
- Hits 71349 65888 -5461
- Misses 30341 35926 +5585
+ Partials 2353 2255 -98 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…PI generation. Enhance tests to verify correct schema behavior for properties with and without nullability. This ensures accurate representation of JSON types in generated schemas.
Member
|
@r-tome I'm OOO shortly, can you please do this review given you created the ticket? Thanks! |
r-tome
reviewed
Aug 17, 2026
…data-on-every-response
r-tome
reviewed
Aug 19, 2026
r-tome
left a comment
r-tome
left a comment
Contributor
There was a problem hiding this comment.
This looks good to me but I designed the solution so I tagged @jrmccannon to review it.
I feel like the comments/xmldoc are too Claude-y. Maybe retry using simplified technical english.
jrmccannon
requested changes
Aug 19, 2026
…ved serialization - Refactored PolicyStatusResponseModel and PolicyResponseModel to ensure consistent serialization behavior, preserving legacy wire format for policies with no stored data. - Updated unit tests to reflect changes in serialization logic, replacing legacy models with dictionary-based deserialization for accurate JSON output comparison. - Removed obsolete RawJsonConverterTests as they are no longer needed with the updated serialization approach.
jrmccannon
requested changes
Aug 20, 2026
…JSON object for missing data - Modified PolicyStatusResponseModel to return an empty JSON object instead of null when no data is stored, ensuring compatibility with policy-specific initialization logic. - Updated comments for clarity regarding serialization behavior. - Adjusted RawJsonConverter and RawJsonSchemaFilter documentation for improved understanding of JSON handling.
jrmccannon
reviewed
Aug 20, 2026
| }, | ||
| "Stripe.net": { | ||
| "type": "Transitive", | ||
| "resolved": "48.5.0", |
Contributor
There was a problem hiding this comment.
Is this supposed to be here?
Contributor
Author
There was a problem hiding this comment.
Accident! Good catch!
…le projects - Upgraded various Microsoft.AspNetCore and Microsoft.Extensions packages to version 10.0.10 in the lock files for Commercial.Core, Commercial.Infrastructure.EntityFramework, Scim, Services.Pam, Sso, and test projects. - Ensured consistent dependency resolution and improved compatibility with the latest features and fixes in the updated packages.
jrmccannon
requested changes
Aug 20, 2026
jrmccannon
left a comment
jrmccannon
left a comment
Contributor
There was a problem hiding this comment.
This is the opposite of what I meant.
…property - Changed the Data property in PolicyResponseModel from private to public set, enabling modification of the property after object instantiation. - This adjustment enhances flexibility in handling policy data within the response model.
…data-on-every-response
…data-on-every-response
jrmccannon
approved these changes
Aug 24, 2026
jrmccannon
left a comment
jrmccannon
left a comment
Contributor
There was a problem hiding this comment.
Thanks for all the clean up. One more thing would be to do a manual test of exporting the API spec to do one final gut check to make sure it outputs the spec correctly. These are used for the sdk api bindings so we should validate that to make sure things are breaking across projects.
JaredScar
deleted the
ac/pm-38329-policies-stop-converting-policy-data-on-every-response
branch
August 24, 2026 17:26
cyprain-okeke
added a commit
that referenced
this pull request
Aug 25, 2026
QA round 3: rendering org names verbatim let mail clients linkify names that look like domains (e.g. "40167.Client.Org" became a link to http://40167.client.org/ in the provider payment method email). Insert a zero-width non-joiner after each "." and "@" via the new CoreHelpers.PreventEmailAutoLinking so the visible text is unchanged but the token no longer matches a domain. Handlebars encodes the character as ‌ in HTML parts; the text templates now use triple-stache for the name fields so the raw character (not the entity) lands in the plain-text part, matching the PM-42262 approach for text-part links.
cyprain-okeke
added a commit
that referenced
this pull request
Aug 27, 2026
* [PM-40167] feat: Update billing domain email copy
Refreshes subject lines and body copy for nine billing emails
(license, seat limits, provider payment method, and Families for
Enterprise sponsorship) per the PM-38898 terminology epic. Copy
ships in-place with no feature flag per product decision.
Also removes the unused offerAcceptanceDate parameter from
SendFamiliesForEnterpriseRemoveSponsorshipsEmailAsync, sanitizes
the sponsor org name in offer emails, and fixes the sponsorship
reverting text template referencing a nonexistent {{Date}} property.
* [PM-42261] fix: Address QA defects on billing domain emails
- PM-42261: render organization names verbatim in the seat limit,
Secrets Manager limit, and sponsorship offer email bodies instead of
applying [dot]/[at] substitution, matching the subject line
- PM-42262: point the provider payment method email at the
billing/payment-details route, update the help article anchor, and
render plain text links as bare URLs instead of HTML anchors or
HTML-encoded ampersands
- PM-42264: inherit BaseMailModel so the Secrets Manager machine
accounts limit email footer renders the copyright year
Also aligns sponsorship offer email test expectations with the
PM-41951 copy that shipped on main and won the merge conflict.
* [PM-42261] fix: Render organization names verbatim in remaining billing emails
QA round 2 found the same [dot] substitution in two more emails changed
by PM-40167: the Families sponsorship removal email and the self-host
license expired email. Render the organization name verbatim in both,
matching the earlier fix, and do the same for the provider payment
method email which had the identical latent issue. Handlebars {{ }}
interpolation still HTML-encodes the values.
* [PM-42261] fix: Stop mail clients auto-linking dotted organization names
QA round 3: rendering org names verbatim let mail clients linkify
names that look like domains (e.g. "40167.Client.Org" became a link to
http://40167.client.org/ in the provider payment method email). Insert
a zero-width non-joiner after each "." and "@" via the new
CoreHelpers.PreventEmailAutoLinking so the visible text is unchanged
but the token no longer matches a domain. Handlebars encodes the
character as ‌ in HTML parts; the text templates now use
triple-stache for the name fields so the raw character (not the
entity) lands in the plain-text part, matching the PM-42262 approach
for text-part links.
* Fix lint whitespace and restore SanitizeForEmail doc placement
The PreventEmailAutoLinking helper was inserted between SanitizeForEmail
and its XML summary, leaving the new method with two summaries and
SanitizeForEmail undocumented. Reorder so each method keeps its own
docs, and fix the missing indentation on the new CoreHelpers test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-38329
📔 Objective
Stops converting the policy data on every response for policies