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

refactor: use hooks to extend bulk_transaction (backport #43058) #43434

Merged

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Sep 30, 2024

Bulk Transaction is very useful feature, but now it is really difficult to add more methods.

When creating a new app, there is a chance that the the new kind of bulk transaction is required.

This PR move the mapper function inside a Doctype so it can be extened by super() in other app (I can't find a proper doctype other than Bulk Transaction Detail Log to place the get_mapper()).

I.e., I have a new app, and so I can do something like this with override_doctype_class.

from erpnext.bulk_transaction.doctype.bulk_transaction_log_detail.bulk_transaction_log_detail import BulkTransactionLogDetail


class BulkTransactionLogDetailThaiPayroll(BulkTransactionLogDetail):
    
    def get_mapper(self):
        from thai_payroll.custom import salary_slip
        mapper = super().get_mapper()
        mapper.update({
            "Salary Slip": {
		    	"Withholding Tax Cert Employee": salary_slip.make_withholding_tax_cert_employee,
		    },
        })
        return mapper

Note: IMHO, writing method within a class should be a preferred way to allow easier extension. Currently, in many case, I always need to do monkey patch and method overwrite (which is not good for upgrade).


This is an automatic backport of pull request #43058 done by Mergify.

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Sep 30, 2024
@ruthra-kumar ruthra-kumar self-assigned this Sep 30, 2024
@ruthra-kumar ruthra-kumar merged commit 6f3b560 into version-15-hotfix Sep 30, 2024
12 checks passed
@ruthra-kumar ruthra-kumar deleted the mergify/bp/version-15-hotfix/pr-43058 branch September 30, 2024 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants