Exporting fields with changed `column_name` doesn't work in v4 about django-import-export HOT 15 CLOSED

@PetrDlouhy - I can't reproduce it exactly in the way you have it.

• What happens if you declare a fields list with the fields you want in the export?
• I have a test case here
  • I find that if I remove the fields declaration, I get all Resource fields in the export.
  • Are you able to reproduce using this test case?

from django-import-export.

@matthewhegarty I think, that test is too specific for to test the actual problem. The test runs only ModelResource.export(), but the problem would probably lie in the admin form handling these field checkboxes:

I expect that it behaves like if the checkbox for a field was not selected.

I will try to investigate further and make the appropriate test.

from django-import-export.

Hi Petr
I tried with your branch but reverted all your changes from resources.py. I added a new EBook and set the author to "Ian Fleming".

When I export the Ebook, I see:

The export file I get is:

id,Email of the author,name,published_date
1,[email protected],Moonraker 2,2024-05-27

So it's possibly a bug that the Export HTML doesn't show the name of the field name to be exported ("Email of the author"), but other than that it looks ok... Am I missing something?

from django-import-export.

@matthewhegarty I didn't touch that part and I am not sure, which variant is correct. The original field name can be more readable for the user as it is/can be consistent across multiple resources.
I can imagine, that some resources would have totally unreadable column_names, e.g. IDs or legal identifiers.

Maybe we should display both values like Author Email (Email of the author).

from django-import-export.

@matthewhegarty I have added that functionality as separate PR #1857. Please check it and decide if you like this style or if I should prefer only one variant.

from django-import-export.

I notice that if I add the attribute name, then the test in #1857 passes:

# test fails
author = fields.Field(column_name="AuthorFooName")

# test passes
author = fields.Field(attribute="author", column_name="AuthorFooName")

This is similar to what was reported here this could be a v4 regression, I'll test in v3.

from django-import-export.

I tested in v3 (added test here). This passes in v3, so we have a regression in v4 when the attribute name is not supplied.

from django-import-export.

@matthewhegarty I don't understand entirely. The test in will pass after applying #1856 (not #1857).

Is there something that needs to be fixed in either of those commits?
But maybe I am missing something, I will look at it tomorrow with fresh mind.

from django-import-export.

Hi, I am still looking into it. I think there are a couple of things going on with other parts of the code which need addressing. I'll write a more detailed response tomorrow.

from django-import-export.

hi Petr
I adapted your changes and made a new PR (#1861). I found that if I set an attribute on the field during initialization, then this resolves the issue without needed to modify resources.py. I have kept your other changes in.

The reason this works is because the author field in test_export_fields_column_name does not contain a defined attribute, therefore it is ignored during export.

This also fixes an issue I uncovered whilst testing (#1860). I propose that I merge this PR instead of yours. What do you think?

from django-import-export.

@matthewhegarty That seems to be a better solution. I tested it and it seems to be working.

The PR is missing the refactoring made in this commit: 9d2cab3
Those changes are unrelated to the PR itself, so it is clear why they are not in the PR (although the commit is and then the changes are reverted, so I suggest it might be better to rebase it out).
I think that the refactoring would be still beneficial for django-import-export and I can make it in separate PR (after merging #1861), if you think it too.

from django-import-export.

I only removed the change so that I could be sure that changes to resources.py did not affect the fix in #1861.

There is some duplication in those methods, so feel free to submit a new PR to make the code clearer. I would recommend to hide any shared logic in a protected method.

I also wasn't comfortable with the new Field attribute called original_name, but we shouldn't need that now due to the change in #1861.

Thanks again for all your help in identifying and fixing this issue 👍

from django-import-export.

Yeah, the original_name was only a tool to transfer the field variable name. Since now it is in attribute we don't need it anymore. I will make the refactor PR after #1861 is merged.

from django-import-export.

@matthewhegarty Everything seems to be working. Thank you very much for your attention and all the work you put into this project.

from django-import-export.

np - thanks for your input!

from django-import-export.