Potential for incomplete data saving from dict value-based Address Model Creation

[banagale]

If you create an address object A using

Address.objects.create(raw='123 MyStreet, Portland, OR 97269'

Then in another model with an AddressField address:

Class Building(models.Model):
    address = AddressField(on_delete=models.SET_NULL, null=True)

Then attempt to set the address field to a more complete form of address object using the value-dict notation:

value = {
    'raw':'123 MyStreet, Portland, OR 97269',
    'formatted': ''123 MyStreet Blvd, Portland, Oregon 97269' ,
    'latitude': '38.8781932',
    'longitude': '-75.8252961',
}

my_building_instance.address = value

The formatted, latitude and longitude values will not be set because an Address with this raw exists. It is also possible for this to occur if an address with unique-together Address object instance of street_number, route and locality exists.

Here's the code in question.

It isn't clear to me why these values would not be set in this circumstance.

These values are saved when no existing partially matched Address object is located. It appears the logic is an attempt to avoid unnecessary creation of existing Address objects.

However, with no warning or errors that an existing address is being returned from the _to_python() method, the package silently drops formatted and the coordinates.

Instead, it is likely safe to assume that if a more complete value-dict is passed in, the existing found object should be updated with these values rather than casting them off.

[banagale]

Beyond this, it is also not very difficult to get into a data integrity issue where there are more than one Address objects that meet the conditionals under Handle the address.

For example, if you have two Address instances of Atlantic Ocean, it will trigger an exception from this package.

[carlosa54]

Hi @banagale,

First I would like to thank you for the follow ups you have made to this repo and adding Django 3.0 support. Adding another use case similar to this issue with raw but could apply to the dict values as well.

If someone is only adding addresses by using raw and wants to edit a specific address the following code shows how it creates unnecessary records for just an edit.

object.address = "Some address with a typo"
object.save()
object.address_id
13
object.address = "Some address"
object.save()
object.address_id
14

This specific case could be solved by changing the address raw field directly like object.address.raw = "Insert edit"

Not sure if it's worth adding support for updating the same address record when setting the raw like this object.address = "Some address" but wanted to bring it up just in case.

[banagale]

@carlosa54

Thank you for your appreciation of my work so far on this package. I'm new at maintainership, so it means a lot to me.

Just as important is this note on this data duplication issue. I agree they are related to the substance of this ticket. I'm curious, how did you come across this yourself?

I wonder if the way to handle this might be:

1. Establish the expected behavior, possibly in a short user story, (which would be the basis of the documentation)
2. Write a test for the behavior
3. Update the code so it passes the test

I've just updated my proposed pathway for this package which puts emphasis on improving what we have, so I think this should be slotted into an upcoming release.

[carlosa54]

@banagale Sorry I didn't get back to you earlier, so I came across this one mostly testing adding addresses using only the raw/string on a personal project. I also encountered the same issue as the main message of this thread, when updating latitude/longitude for an existing address if I send it as a dictionary it won't update. As a workaround instead of sending the dict I'm just updating it directly.

From this:

address_dict = { ..., 'latitude': 'some_lat', 'longitude': 'some_lng' }
my_model.address = address_dict

To this:

address_dict = { ..., 'latitude': 'some_lat', 'longitude': 'some_lng' }
address.latitude = address_dict.get('latitude')
address.longitude = address_dict.get('longitude')
address.save()

[banagale]

Thanks, @carlosa54. I've designated this a high priority issue.

[banagale]

I noticed that #13 mentions the need to offer address updates using raw. It is not obvious to me yet what needs to be updated in the admin of the field once this is corrected. Will keep an eye on it.

---

Stubbing Documentation for Behavior:

Setting Address Values

You can set an Address object instance's values two ways:

1. Using raw values
   [Show example, including how locality will be created]
   [Explain behavior]
2. Using an existing valid Address object
   [Show example(s) including Locality if necessary]

---