Commits

Luke Plant committed b2cc032

Made ManyToManyFilter consistent with ForeignKeyFilter in using model instances as choice objects

Involved some cleanup to API. Also ensured no performance regressions

Comments (0)

Files changed (2)

django_easyfilters/filters.py

 
     ### Utility methods needed by most/all subclasses ###
 
-    def param_from_choices(self, choices):
+    def param_from_choice(self, choice):
+        return unicode(choice)
+
+    def paramlist_from_choices(self, choices):
         """
         For a list of choices, return the parameter list that should be created.
         """
-        return map(unicode, choices)
+        return map(self.param_from_choice, choices)
 
     def build_params(self, add=None, remove=None):
         """
             if add not in chosen:
                 chosen.append(add)
         if chosen:
-            params.setlist(self.query_param, self.param_from_choices(chosen))
+            params.setlist(self.query_param, self.paramlist_from_choices(chosen))
         else:
             del params[self.query_param]
         params.pop('page', None) # links should reset paging
             raise ValueError("object does not exist in DB")
         return obj
 
+    def param_from_choice(self, choice):
+        return unicode(choice.pk)
+
     def get_choices_add(self, qs):
         count_dict = self.get_values_counts(qs)
         lookup = {self.rel_field.name + '__in': count_dict.keys()}
             pk = getattr(o, self.rel_field.attname)
             choices.append(FilterChoice(self.render_choice_object(o),
                                         count_dict[pk],
-                                        self.build_params(add=pk),
+                                        self.build_params(add=o),
                                         FILTER_ADD))
         return choices
 
 
         return [FilterChoice(self.render_choice_object(o),
                              count_dict[o.pk],
-                             self.build_params(add=o.pk),
+                             self.build_params(add=o),
                              FILTER_ADD)
                 for o in objs]
 
-    def get_choices_remove(self, qs):
-        chosen = self.chosen
-        # Do a query in bulk to get objs corresponding to choices.
-        objs = self.rel_model.objects.filter(pk__in=chosen)
+    def param_from_choice(self, choice):
+        return unicode(choice.pk)
 
-        # We want to preserve order of items in params, so use the original
-        # 'chosen' list, rather than objs.
+    def choices_from_params(self):
+        # To create the model instances, we override this method rather than
+        # choice_from_param in order to do a single bulk query rather than
+        # multiple queries. So 'choice_from_param' technically returns the
+        # wrong type of thing, since it returns PKs not instances.
+        chosen_pks = super(ManyToManyFilter, self).choices_from_params()
+        objs = self.rel_model.objects.filter(pk__in=chosen_pks)
+        # Now need to get original order back. But also need to be aware
+        # that some things may not exist in DB
         obj_dict = dict([(obj.pk, obj) for obj in objs])
-        return [FilterChoice(self.render_choice_object(obj_dict[choice]),
-                             None, # Don't need count for removing
-                             self.build_params(remove=[choice]),
-                             FILTER_REMOVE)
-                for choice in chosen if choice in obj_dict]
+        retval = []
+        for c in chosen_pks:
+            if c in obj_dict:
+                retval.append(obj_dict[c])
+        return retval
 
 
 class DateRangeType(object):

django_easyfilters/tests/filterset.py

         # If we select 'emily' as an author:
 
         data =  MultiValueDict({'authors':[str(emily.pk)]})
-        filter1 = ManyToManyFilter('authors', Book, data)
-        qs_emily = filter1.apply_filter(qs)
+        with self.assertNumQueries(1):
+            # 1 query for all chosen objects
+            filter1 = ManyToManyFilter('authors', Book, data)
+
+        with self.assertNumQueries(0):
+            # This shouldn't need to do any more queries
+            qs_emily = filter1.apply_filter(qs)
 
         # ...we should get a qs that includes Poems and Wuthering Heights.
         self.assertTrue(qs_emily.filter(name='Poems').exists())
         # ...and excludes Jane Eyre
         self.assertFalse(qs_emily.filter(name='Jane Eyre').exists())
 
-        # We should get a 'choices' that includes charlotte and anne
-        choices = filter1.get_choices(qs_emily)
+        with self.assertNumQueries(2):
+            # 0 query for all chosen objects (already done)
+            # 1 query for available objects
+            # 1 query for counts
+            choices = filter1.get_choices(qs_emily)
+
+        # We should have a 'choices' that includes charlotte and anne
         self.assertTrue(unicode(anne) in [c.label for c in choices if c.link_type is FILTER_ADD])
         self.assertTrue(unicode(charlotte) in [c.label for c in choices if c.link_type is FILTER_ADD])
 
         # emily should be in 'remove' links, however.
         self.assertTrue(unicode(emily) in [c.label for c in choices if c.link_type is FILTER_REMOVE])
 
-        # If we select again:
-        data2 =  MultiValueDict({'authors': [str(emily.pk), str(anne.pk)]})
-        filter2 = ManyToManyFilter('authors', Book, data2)
+        # Select again - should have sensible params
+        anne_choice = [c for c in choices if c.label.startswith('Anne')][0]
+        self.assertTrue(unicode(emily.pk) in anne_choice.params.getlist('authors'))
+        self.assertTrue(unicode(anne.pk) in anne_choice.params.getlist('authors'))
+
+        # Now do the second select:
+        filter2 = ManyToManyFilter('authors', Book, anne_choice.params)
 
         qs_emily_anne = filter2.apply_filter(qs)