Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add field optional dehydrate method param #1536

Conversation

cocorocho
Copy link
Contributor

Problem

Defining custom methods for dehydration of the field

I have had situations where I had to dynamically generate new fields during export. In this case I wasn't able to pre-define dehydration methods in ModelResource so I added dehydrate_method parameter to Field which solved this issue.

Solution

I have added dehydrate_method parameter to fields.Field so custom methods

Acceptance Criteria

Have you written tests? Have you included screenshots of your changes if applicable?

I have written tests.

Did you document your changes?
Added docstrings.

Let me know if anything needs to be changed!

@coveralls
Copy link

coveralls commented Feb 4, 2023

Coverage Status

Coverage: 100.0%. Remained the same when pulling 4c8ecab on cocorocho:add-field-optional-dehydrate-method-param into 51d703b on django-import-export:main.

Copy link
Contributor

@matthewhegarty matthewhegarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this - looks great. I had a couple of minor comments.

import_export/fields.py Outdated Show resolved Hide resolved
import_export/fields.py Outdated Show resolved Hide resolved
@cocorocho
Copy link
Contributor Author

I have made the changes.

Also deleted the deprecation warning from exceptions module.

Thanks for the suggestions and explanations regarding the changes, if anything else needs to be changed let me know!

@matthewhegarty
Copy link
Contributor

Looks great, almost ready to merge. Re backticks, I suggest remove these from the FieldError message (sorry I was not clear about that). Error messages can be written to log files and although I cannot see how, I wonder if having backticks could be exploitable, therefore I suggest to use single quotes. Please could you correct it?

Feel free to add your name to AUTHORS (optional), and then I will merge.

@cocorocho
Copy link
Contributor Author

Looks great, almost ready to merge. Re backticks, I suggest remove these from the FieldError message (sorry I was not clear about that). Error messages can be written to log files and although I cannot see how, I wonder if having backticks could be exploitable, therefore I suggest to use single quotes. Please could you correct it?

Feel free to add your name to AUTHORS (optional), and then I will merge.

Ah lol ok now it makes sense. Will make these changes

@matthewhegarty
Copy link
Contributor

Great thanks

@matthewhegarty matthewhegarty merged commit 09dc284 into django-import-export:main Feb 7, 2023
@cocorocho cocorocho deleted the add-field-optional-dehydrate-method-param branch February 7, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants