feat(relations): save with update_fields

[koeaw]

When Property's save method is called with
update_fields to update any of the name fields,
make sure both name fields are considered when
saving.

[koeaw]

I'd consider this a fix but it doesn't fix existing code but adds something new, so... unsure which commit type to use.

[b1rger]

I'd consider this a fix but it doesn't fix existing code but adds something new, so... unsure which commit type to use.

I'd use fix - simply because feat leads to a version bump.

I think the snippet is a bit hard to read - I understand the faszination with the walrus operator, but in this case it makes it hard to follow the logic (at least for me). Wouldn't something like this be more readable?:

name_fields = {"name_forward", "name_reverse"}
update_fields = kwargs.get("update_fields", [])
if name_fields.intersection(update_fields):
    kwargs["update_fields"] = set(update_fields) | include_fields

(But maybe thats missing a corner case, the original code works for ...)

[koeaw]

I'd use fix - simply because feat leads to a version bump.

Ah, I wasn't aware (or possibly had forgotten), good to know. Will change it.

I think the snippet is a bit hard to read - I understand the faszination with the walrus operator

Guilty as charged. XD I'll simplify it.

[koeaw]

I realised that with deprecated_name having got dropped, the only remaining name field that's dependent on another is name_reverse, so set comparison via intersection has become superfluous.

I kept one walrus operator in this adaptation because I think it is easy enough to read/understand the way it's used here; the "long form" version I would have gone with resulted in forced reformatting of the if statement, which I didn't think helped readability either.

[b1rger]

Oke, but now we could simply not use a Set at all, right?

 if (update_fields := kwargs.get("update_fields")) is not None:
     if "name_forward" in update_fields and "name_reverse" not in update_fields:
         kwargs["update_fields"].append("name_reverse")

[koeaw]

Oke, but now we could simply not use a Set at all, right?

update_fields can be set to any iterable which can hold strings, so converting to set is the most universal approach, I believe. (Inclulding if it were set to just a string, I think?)

Docs

[b1rger]

update_fields can be set to any iterable which can hold strings, so converting to set is the most universal approach, I believe. (Inclulding if it were set to just a string, I think?)

ah, oke!