MarkusH / django-osm-field

http://markush.github.io/django-osm-field
MIT License
29 stars 12 forks source link

Fix for OSMWidget in formsets #9

Closed blag closed 8 years ago

blag commented 8 years ago

Fixes #7.

Got the idea from this StackOverflow post.

MarkusH commented 8 years ago

I feel like there has to be another way to get the prefix out of a form in a formset. You might want to look at django.forms for how it's implemented there.

blag commented 8 years ago

I reworked this PR so it no longer requires any customization of the form itself and rebased it on #11 and #8.

I created a OSMFormField in forms.py and moved OSMWidget into widgets.py, so imports will need to be changed for users upgrading to this. I can add reverse-compatibility shims into it if necessary.

blag commented 8 years ago

Done.

codecov-io commented 8 years ago

Current coverage is 7.56%

Merging #9 into develop will decrease coverage by 80.75%

@@            develop        #9   diff @@
=========================================
  Files             5         6     +1   
  Lines           154       172    +18   
  Methods           0         0          
  Messages          0         0          
  Branches         14        16     +2   
=========================================
- Hits            136        13   -123   
- Misses           12       159   +147   
+ Partials          6         0     -6   

Powered by Codecov. Last updated by 2ef0aaa...c9320ab

blag commented 8 years ago

As far as I've been able to find, for django < 1.9 there's no way to pass a form's prefix (eg: in a formset) to each widget individually. To work around this, I created a form mixin that adds the form's prefix to each OSMFormField field:

class OSMFormMixin(object):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        if django.VERSION < (1, 9):
            for k in self.fields:
                f = self.fields[k]
                if issubclass(f.__class__, OSMFormField):
                    f.widget.attrs['prefix'] = self.prefix

This will also work for Django >= 1.9, but it's unnecessary. I have updated the usage documentation to reflect this.

blag commented 8 years ago

Okay, reworked everything:

So this changeset looks ready to go as far as I can tell. Let me know if you would like me to do anything else for it. Thanks!

blag commented 8 years ago

Bump. Let me know if there's anything else you'd like me to do for this PR.

And just FYI: I plan on rebasing my other PRs off of master as you merge their preceding PRs in to minimize their diffs and ease review. :smile: