Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/479.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow unsetting optional relationship of cardinality one by setting its value to `None`
4 changes: 3 additions & 1 deletion infrahub_sdk/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
VARIABLE_TYPE_MAPPING = ((str, "String!"), (int, "Int!"), (float, "Float!"), (bool, "Boolean!"))


def convert_to_graphql_as_string(value: str | bool | list | BaseModel | Enum | Any, convert_enum: bool = False) -> str: # noqa: PLR0911
def convert_to_graphql_as_string(value: Any, convert_enum: bool = False) -> str: # noqa: PLR0911
if value is None:
return "null"
if isinstance(value, str) and value.startswith("$"):
return value
if isinstance(value, Enum):
Expand Down
28 changes: 17 additions & 11 deletions infrahub_sdk/node/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,10 @@ def _generate_input_data( # noqa: C901

rel: RelatedNodeBase | RelationshipManagerBase = getattr(self, item_name)

# BLOCKED by https://github.com/opsmill/infrahub/issues/330
# if (
# item is None
# and item_name in self._relationships
# and self._schema.get_relationship(item_name).cardinality == "one"
# ):
# data[item_name] = None
# continue
# el
if rel_schema.cardinality == RelationshipCardinality.ONE and rel_schema.optional and not rel.initialized:
data[item_name] = None
continue

if rel is None or not rel.initialized:
continue

Expand Down Expand Up @@ -315,7 +310,16 @@ def _strip_unmodified_dict(data: dict, original_data: dict, variables: dict, ite
variables.pop(variable_key)

# TODO: I do not feel _great_ about this
if not data_item and data_item != [] and item in data:
# -> I don't even know who you are (but this is not great indeed) -- gmazoyer (quoting Thanos)
original_data_item = original_data.get(item)
original_data_item_is_none = original_data_item is None
if isinstance(original_data_item, dict):
if "node" in original_data_item:
original_data_item_is_none = original_data_item["node"] is None
elif "id" not in original_data_item:
original_data_item_is_none = True

if item in data and (data_item in ({}, []) or (data_item is None and original_data_item_is_none)):
data.pop(item)

def _strip_unmodified(self, data: dict, variables: dict) -> tuple[dict, dict]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole "strip unmodified" code feels difficult to read. I think we probably want to take on the burden of trying to improve it to make it easier to maintain.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, we should probably look at the schema of the node as an initial starting point when refactoring and figure out what we want this to look like.

Expand All @@ -324,7 +328,9 @@ def _strip_unmodified(self, data: dict, variables: dict) -> tuple[dict, dict]:
relationship_property = getattr(self, relationship)
if not relationship_property or relationship not in data:
continue
if not relationship_property.initialized:
if not relationship_property.initialized and (
not isinstance(relationship_property, RelatedNodeBase) or not relationship_property.schema.optional
):
data.pop(relationship)
elif isinstance(relationship_property, RelationshipManagerBase) and not relationship_property.has_update:
data.pop(relationship)
Expand Down
6 changes: 6 additions & 0 deletions tests/integration/test_infrahub_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ async def test_create_generic_rel_with_hfid(
person_sophia = await client.get(kind=TESTING_PERSON, id=person_sophia.id, prefetch_relationships=True)
assert person_sophia.favorite_animal.id == cat_luna.id

# Ensure that nullify it will remove the relationship related node
person_sophia.favorite_animal = None
await person_sophia.save()
person_sophia = await client.get(kind=TESTING_PERSON, id=person_sophia.id, prefetch_relationships=True)
assert not person_sophia.favorite_animal.id

async def test_task_query(self, client: InfrahubClient, base_dataset, set_pagination_size3) -> None:
nbr_tasks = await client.task.count()
assert nbr_tasks
Expand Down
15 changes: 10 additions & 5 deletions tests/unit/sdk/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,12 @@ async def test_create_input_data(client, location_schema: NodeSchemaAPI, client_
node = InfrahubNodeSync(client=client, schema=location_schema, data=data)

assert node._generate_input_data()["data"] == {
"data": {"name": {"value": "JFK1"}, "description": {"value": "JFK Airport"}, "type": {"value": "SITE"}}
"data": {
"name": {"value": "JFK1"},
"description": {"value": "JFK Airport"},
"type": {"value": "SITE"},
"primary_tag": None,
}
}


Expand Down Expand Up @@ -1577,7 +1582,7 @@ async def test_create_input_data_with_IPHost_attribute(client, ipaddress_schema,
ip_address = InfrahubNodeSync(client=client, schema=ipaddress_schema, data=data)

assert ip_address._generate_input_data()["data"] == {
"data": {"address": {"value": "1.1.1.1/24", "is_protected": True}}
"data": {"address": {"value": "1.1.1.1/24", "is_protected": True}, "interface": None}
}


Expand All @@ -1591,7 +1596,7 @@ async def test_create_input_data_with_IPNetwork_attribute(client, ipnetwork_sche
ip_network = InfrahubNodeSync(client=client, schema=ipnetwork_schema, data=data)

assert ip_network._generate_input_data()["data"] == {
"data": {"network": {"value": "1.1.1.0/24", "is_protected": True}}
"data": {"network": {"value": "1.1.1.0/24", "is_protected": True}, "site": None}
}


Expand Down Expand Up @@ -1789,7 +1794,7 @@ async def test_update_input_data_empty_relationship(
"data": {
"id": "llllllll-llll-llll-llll-llllllllllll",
"name": {"value": "DFW"},
# "primary_tag": None,
"primary_tag": None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having these should be alright. This does not seem to have a negative impact.

"tags": [],
"type": {"value": "SITE"},
},
Expand All @@ -1798,7 +1803,7 @@ async def test_update_input_data_empty_relationship(
"data": {
"id": "llllllll-llll-llll-llll-llllllllllll",
"name": {"is_protected": True, "is_visible": True, "value": "DFW"},
# "primary_tag": None,
"primary_tag": None,
"tags": [],
"type": {"is_protected": True, "is_visible": True, "value": "SITE"},
},
Expand Down