barseghyanartur / django-elasticsearch-dsl-drf

Integrate Elasticsearch DSL with Django REST framework.
https://pypi.python.org/pypi/django-elasticsearch-dsl-drf
367 stars 117 forks source link

Allow more specific targeting for nested sort fields. #212

Closed helrond closed 3 years ago

helrond commented 4 years ago

Is your feature request related to a problem? Please describe. The current implementation of nested sorting presumed that all Nested fields are part of another Nested field. Therefore, if the path value for an ordering field is something like foo.bar, both foo and bar must be Nested fields.

Describe the solution you'd like I believe this is the because of the way the nested_sort_entry function in compat.py is implemented. I think the solution would be to add an optional parameter to this function which would just return the full path rather than splitting it on line 81.

Happy to submit a pull request for this.

barseghyanartur commented 4 years ago

PR would be great. Please, make sure that added functionality is tested (update tests).

helrond commented 4 years ago

Thanks @barseghyanartur - I have made the changes in the code but wanted to confirm the strategy for updating tests before I got into that.

From what I am seeing, it looks like I will need to update the models and documents at /examples/simple to add an ObjectField which contains a NestedField. I will then need to add additional tests in /src/tests/test_ordering_common.py which test ordering on that new field. Does that sound right? Are there any steps I'm missing?

barseghyanartur commented 4 years ago

Yep. Something like that. Please, do not change existing fields. Rather add new ones.

Before proceding, please, check this:

helrond commented 3 years ago

OK I've spent a fair amount of time the last couple of days digging into this and have made a bunch of updates. I'm getting an error (stack trace below) when attempting to run tests, and I have no idea how to resolve this because I've never worked with Faker before and the documentation isn't giving me any hints. What am I doing wrong?

diff is here

___________________________________________________ ERROR at setup of TestOrdering.test_schema_fields_with_filter_fields_list ____________________________________________________

self = <faker.generator.Generator object at 0x110075d10>, formatter = 'factories.books_planet.PlanetFactory'

    def get_formatter(self, formatter):
        try:
>           return getattr(self, formatter)
E           AttributeError: 'Generator' object has no attribute 'factories.books_planet.PlanetFactory'

../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/faker/generator.py:79: AttributeError

During handling of the above exception, another exception occurred:

cls = <class 'django_elasticsearch_dsl_drf.tests.test_ordering_common.TestOrdering'>

    @classmethod
    def setUpClass(cls):
        """Set up class."""
        super(TestOrdering, cls).setUpClass()

        cls.address_in_sydney = factories.AddressFactory.create(
            **{
                'city__name': 'Sydney',
                'city__country__name': 'Australia',
>               'city__country__continent__name': 'Australia',
            }
        )

src/django_elasticsearch_dsl_drf/tests/test_ordering_common.py:76: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/base.py:563: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/base.py:500: in _generate
    return step.build()
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/builder.py:272: in build
    step.resolve(pre)
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/builder.py:221: in resolve
    self.attributes[field_name] = getattr(self.stub, field_name)
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/builder.py:375: in __getattr__
    extra=context,
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/faker.py:56: in evaluate
    return self.generate(extra or {})
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/factory/faker.py:53: in generate
    return subfaker.format(self.provider, **kwargs)
../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/faker/generator.py:75: in format
    return self.get_formatter(formatter)(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <faker.generator.Generator object at 0x110075d10>, formatter = 'factories.books_planet.PlanetFactory'

    def get_formatter(self, formatter):
        try:
            return getattr(self, formatter)
        except AttributeError:
            if 'locale' in self.__config:
                msg = 'Unknown formatter "{0}" with locale "{1}"'.format(
                    formatter, self.__config['locale'],
                )
            else:
                raise AttributeError('Unknown formatter "{0}"'.format(
                    formatter,
                ))
>           raise AttributeError(msg)
E           AttributeError: Unknown formatter "factories.books_planet.PlanetFactory" with locale "nl_NL"

../../.pyenv/versions/3.7.8/lib/python3.7/site-packages/faker/generator.py:89: AttributeError
barseghyanartur commented 3 years ago

@helrond:

Sorry for coming back so late to this. Your branch works well for me (no tests failing). Perhaps it's a matter of version pinning.

This works for me:

factory-boy                        2.11.1 
Faker                              0.8.17

Do not hesitate to make a PR.

barseghyanartur commented 3 years ago

Released in 0.22