search

diefenbach / django-lfs

An online-shop based on Django
http://www.getlfs.com
BSD 3-Clause "New" or "Revised" License
622 stars 222 forks source link

allow saving of addresses that don't have zip codes #102

Closed mthornhill closed 11 years ago

mthornhill commented 11 years ago

Hi Kai, I've added a similar fallback for a missing State, do you think this approach is ok?

diefenbach commented 11 years ago

I think that's at least better than just using the default "None" of the get method.

mthornhill commented 11 years ago

Hi Kai, I was just thinking that it might be a more elegant solution to put a default="" on the definition of the model field, what do you think?

diefenbach commented 11 years ago

Me too.

mthornhill commented 11 years ago

I've just rebased that branch to remove the previous code and added the default=u"" for zip_code, it's not necessary for state as it has null=True, blank=True on the model definition

diefenbach commented 11 years ago

Okay, can you submit a new PR?

mthornhill commented 11 years ago

Submitted here, https://github.com/diefenbach/django-lfs/pull/104

On Fri, Jul 12, 2013 at 4:48 PM, Kai Diefenbach notifications@github.comwrote:

Okay, can you submit a new PR?

— Reply to this email directly or view it on GitHubhttps://github.com/diefenbach/django-lfs/pull/102#issuecomment-20885064 .

p: +353 87 6124111 e: michael@maithu.com

diefenbach commented 11 years ago

Hi Michael,

we should revert this, shouldn't we?

https://github.com/mthornhill/django-lfs/blob/58903fd85fb53b750dd995b298ac1b52d002f476/lfs/addresses/utils.py#L133

mthornhill commented 11 years ago

Hi Kai I think so, we should also revert the line for zip code as the problem is fixed by the database default and n/a isn't very multilingual Shall I create a pull request? Thanks Michael

On 16 Jul 2013, at 07:30, Kai Diefenbach notifications@github.com wrote:

Hi Michael,

we should revert this, shouldn't we?

https://github.com/mthornhill/django-lfs/blob/58903fd85fb53b750dd995b298ac1b52d002f476/lfs/addresses/utils.py#L133

— Reply to this email directly or view it on GitHub.

diefenbach commented 11 years ago

Yes, please.

Kai

mthornhill commented 11 years ago

https://github.com/diefenbach/django-lfs/pull/105

On Tue, Jul 16, 2013 at 8:22 AM, Kai Diefenbach notifications@github.comwrote:

Yes, please.

Kai

— Reply to this email directly or view it on GitHubhttps://github.com/diefenbach/django-lfs/pull/102#issuecomment-21025380 .

p: +353 87 6124111 e: michael@maithu.com