fix get_import_resource_class refactor

[manelclos]

Problem

This commit produced a regression in one of my applications. The method get_resource_class is no longer called on the ModelAdmin.

Solution

Use original code, self was changed for super() in the refactor.

Acceptance Criteria

Local testing only. Will add tests if the change makes sense.

[coveralls]

Coverage remained the same at 98.06% when pulling 5cf693c on fix-get-resource-class-regression into 6d15fe9 on main.

[matthewhegarty]

Thanks for raising @manelclos. Can you help me figure out why it isn't working?

In 2.5.0, we have those methods in ImportMixin:

class ImportMixin(ImportExportMixinBase):

    # ...

    def get_resource_class(self):
        """Returns ResourceClass"""
        if not self.resource_class:
            return modelresource_factory(self.model)
        else:
            return self.resource_class

    def get_import_resource_class(self):
        """
        Returns ResourceClass to use for import.
        """
        return self.get_resource_class()

But now in main they are in mixin super classes:

class BaseImportExportMixin:
    formats = base_formats.DEFAULT_FORMATS
    resource_class = None

    def get_resource_class(self):
        if not self.resource_class:
            return modelresource_factory(self.model)
        return self.resource_class

    def get_resource_kwargs(self, request, *args, **kwargs):
        return {}


class BaseImportMixin(BaseImportExportMixin):
    def get_import_resource_class(self):
        """
        Returns ResourceClass to use for import.
        """
        return super().get_resource_class()

However it has caused a regression for you, so are you able to say exactly what is causing the issue?

thanks

[manelclos]

Yes, see the commit in the PR. The code was previously calling self.get_resource_class() and that was changed to super().get_resource_class().

[matthewhegarty]

Sorry, I must be missing something. I would just like to understand why it is an issue so that I can fix any other occurrences if there are any. I ran an import with both v2.5.0 and main and they execute the same code (i.e the path I posted above). The current 'main' version calls super().get_resource_class() but that calls BaseImportExportMixin.get_resource_class() which is equivalent to v2.5.0.

The method get_resource_class is no longer called on the ModelAdmin.

Presumably if you have overridden get_resource_class() in your ModelAdmin implementation then it doesn't get called - is that the issue?

[manelclos]

The method get_resource_class is no longer called on the ModelAdmin.

Presumably if you have overridden get_resource_class() in your ModelAdmin implementation then it doesn't get called - is that the issue?

Yes, that is exactly it. I use the ImportMixin in a ModelAdmin. I was using the 2.5.0 release and when testing your PR, which includes the refactor commit, I noticed the behaviour change. I tracked it down to the refactor commit and while reviewing the code changes I found the self to super() change that I want to revert.

Sorry for not explaining it better in the first place.

[matthewhegarty]

👍 Thanks for clarifying. I will look into this tomorrow and add some tests for other cases where this will happen.

[matthewhegarty]

I've added some tests and also fixed a couple of other instances of the same issue. I checked the other mixins to confirm that the issue is not duplicated there as well.

Also, I attempted to fix the issue of Github actions running twice.

[manelclos]

Awesome! I've squashed my commit into yours. This is ready for merge now.

[matthewhegarty]

I set the coveralls decrease threshold for failure to 0.05% to fix the failing checks (details here).

[matthewhegarty]

👍