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

[release/7.0] Add back setter for scaffolded collection navigations #30343

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

ajcvickers
Copy link
Member

Port of #30311
Fixes #30286

Description

The generated code when reverse engineering (scaffolding) from an existing database was changed to drop the setter from collection navigation properties. The setter is not needed for EF use cases, so this made the generated code cleaner. However, it turns out that:

  • Like to use the setter to set an entirely new collection, even though it isn't really designed to be used that way
  • Some serializers, notably System.Text.Json, ignore properties without setters, even though the property is pre-initialized and so is never null or needs setting.

Customer impact

Multiple customers prefer the previously generated code, and, more importantly, System.Text.Json no longer serializes entities correctly. There is a workaround--customize the T4 template to add the setter.

How found

Multiple customer reports on 7.0.

Regression

Maybe, if you squint. This isn't a runtime code change; it requires the user to generate new code. However, the code generated is different than it used to be.

Testing

Tests updated.

Risk

Low. No quirk, because this is tooling code. However, the T4 template can be customized to generate whatever the user wants.

@ajcvickers ajcvickers requested a review from a team February 25, 2023 15:00
@ajcvickers ajcvickers added this to the 7.0.x milestone Feb 25, 2023
@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 25, 2023

Maybe add docs about how to revert to the current "no setter" state?

Copy link
Contributor

@bricelam bricelam left a comment

Choose a reason for hiding this comment

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

:shipit:

@rbhanda rbhanda modified the milestones: 7.0.x, 7.0.5 Feb 28, 2023
@wtgodbe wtgodbe merged commit 08fd50b into release/7.0 Mar 7, 2023
@wtgodbe wtgodbe deleted the ButButButButOhWell0225 branch March 7, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants