IFC-2458: Update profile and template on changing unique attribute#9194
Conversation
github-actions
Bot
added
the
group/backend
github-actions
Bot
added
the
type/documentation
There was a problem hiding this comment.
they were automatically generated, and the generated/ is not in .gitignore
There was a problem hiding this comment.
what are they? should they be included?
| # When a user removes unique=True from their schema definition, the uniqueness_constraints | ||
| # are rebuilt from scratch (not inherited). Replicate that by removing the stale entry. | ||
| # Constraint paths use the "attr__value" format (e.g. "nbr_seats__value"), so we must | ||
| # check startswith rather than an exact string match. | ||
| if new_car_schema.uniqueness_constraints: | ||
| new_car_schema.uniqueness_constraints = [ | ||
| paths | ||
| for paths in new_car_schema.uniqueness_constraints | ||
| if not any(p == "nbr_seats" or p.startswith("nbr_seats__") for p in paths) | ||
| ] or None |
There was a problem hiding this comment.
can you just set new_car_schema.uniqueness_constraints to whatever it should be instead of doing it programatically? it seems like this should be a known value
| ) -> SchemaBranch: | ||
| """TestDevice (generate_template=True) has COMPONENT TestPart (generate_template=False). | ||
| serial_number starts unique=True so it is NOT on template instances.""" | ||
| part = _PART.model_copy(deep=True) |
There was a problem hiding this comment.
I believe our duplicate() method would work here
| return registry.schema.register_schema(schema=SchemaRoot(nodes=[_DEVICE, part]), branch=default_branch.name) | ||
|
|
||
|
|
||
| async def test_migration_unique_enable_support_sub_template( |
There was a problem hiding this comment.
I think this case deserves to be in an integration test, either a new one or an existing one. it should ensure that the template schema and instance correctly have the expected attribute added/removed when the attribute's uniqueness is updated
ajtmccarty
left a comment
ajtmccarty
left a comment
There was a problem hiding this comment.
this looks good. I think it is ready to merge once those generated/ files are addressed
| @@ -0,0 +1 @@ | |||
| Changing `unique` to `False` on a schema attribute now correctly triggers a migration that adds the attribute to existing profile and template nodes. | |||
There was a problem hiding this comment.
we try to say objects instead of nodes
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 9 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. This PR modifies the schema migration trigger for the 'unique' attribute, which is a core business logic change affecting data integrity and migration behavior, so a human review is warranted to ensure correctness and completeness, especially since the test coverage only addresses one direction...
Re-trigger cubic
solababs
deleted the
sb-08052026-fix-update-attribute-does-not-affect-template-schema-ifc-2458
branch
2 participants
Why
This PR ensures that removing
unique: Truefrom a schema attribute correctly triggers theAttributeSupportsGeneratedSchemaMigration, which adds the attribute to all existing profile and template node instances.Closes #8894
How to test
Checklist
changelog/8894.fixed.md)