search

burke-software / django-mass-edit

Make bulk changes in the Django admin interface
152 stars 67 forks source link

Find all overrided properties, even in parent classes #45

Closed bourivouh closed 8 years ago

bourivouh commented 9 years ago

There is some problems in current releas. If admin_obj isn't inherited directly from admin.ModelAdmin, mass_edit admin misses properties overrided in base class (or classes). And also mass_admin doesn't use overrided form class from admin_obj.

Those PR trying to fix them.

bufke commented 8 years ago

Sorry for the delay. It's a bit hard maintaining this project since I didn't create it originally. Could you add a unit test proving this pull fixes this? We should slowly start unit testing this project. Thanks.

bourivouh commented 8 years ago

Sure. I'll try to write tests as soon as possible.

bourivouh commented 8 years ago

I'm trying to write tests for all django version since 1.4, cause I saw this comment But actually it's not compatible with django < 1.6. For example meta.model_name, which is used here and here, or get_queryset method (used here) was added only in 1.6 version.

Is it important to make mass admin compatible django < 1.6?

bufke commented 8 years ago

I have no personal interest in supporting Django 1.4 which has an end of life Oct 1. If you'd like to do the work of supporting it and you add unit tests and have travis CI test against a matrix of django 1.4, 1.8 with python 2.7 and 3.4 I would accept the pull request for such and I'm sure others would be thankful. That should go against a separate pull request since I don't believe it's related to this. Right now given how little tests this project has - it's going to be hard to manage compatibility.

Honestly if you had the time a rewrite might be in order. The codebase is largely one big mass_change_view function that isn't going to play well with unit testing.

bourivouh commented 8 years ago

Well, I wrote tests for django 1.6 - 1.8 and both versions of python. Vut something strange happend with my merge. I'm trying to fix it.

I have no interest in 1.4 compatibility too, so don't think that I'll do that. Anyway, I'd like to try refactor mass_change_view, but can't promise to do it soon.

bourivouh commented 8 years ago

omg, I don't know what to do with all this mess. maybe just create a new pull requests?

bufke commented 8 years ago

lol. Up to you. Certainly the best practice is making small pull requests that do one thing. If you want to just consider this a partial rewrite and keep it as is I'd be fine with that. When it's ready to look at I can check it out and make sure it works on my use cases.

bourivouh commented 8 years ago

Hi! I think, I am done with this. Could you review, please?

bufke commented 8 years ago

This looks good. More tests. I didn't see it breaking anything. Thanks.