Use get_class for oscar_invoice.utils.InvoiceCreator

[specialunderwear]

Currently overriding oscar_invoice.utils.InvoiceCreator is non trivial because it is not loaded with get_class. The only way to make this possible is to add one more namespace to the application. Let's say we add a namespace called 'poepie' making the folder structure like this:

oscar_invoices/poepie/utils.py

that would allow using get_class like this:

InvoiceCreator = get_class("poepie.utils", "Invoice_creator", "oscar_invoices")

You can not use get_class with only 2 namespace levels, you CAN however use get_model (it is implemented completely different).

Please suggest the 3rd namespace name and I will create a PR.

[samitnuk]

I think we can add the ability to set custom InvoiceCreator from settings as we did for custom invoice model.

django-oscar-invoices/oscar_invoices/app_settings.py

Line 7 in e7c4676

OSCAR_INVOICES_INVOICE_MODEL = getattr(settings, 'OSCAR_INVOICES_INVOICE_MODEL', 'oscar_invoices.Invoice')

@sasha0 @specialunderwear WDYT?

[sasha0]

You can not use get_class with only 2 namespace levels

Not ideal, but I found a workaround:

get_class("oscar_invoices.utils", "InvoiceCreator")

This way Oscar will try to load class from the local module.

[specialunderwear]

That seems pretty ok to me

[specialunderwear]

Hmm sorry, but how can you override the InvoiceCreator if we import like that?

Should I make a package like:
test/oscar_invoices/utils.py

and put

test.oscar_invoices into INSTALLED_APPS?

[sasha0]

I've got an idea to introduce a new setting and load InvoiceCreator based on it:

OSCAR_INVOICES_INVOICE_CREATOR = getattr(settings, 'OSCAR_INVOICES_INVOICE_CREATOR', 'oscar_invoices.InvoiceCreator')

WDYT @samitnuk @specialunderwear ?

[specialunderwear]

Hi we allready use the current method, which works in our stack. I don't think having 2 different methods would be a good idea. BTW are you guys both having the same idea? Or are you trying to push this feature???

[sasha0]

BTW are you guys both having the same idea? Or are you trying to push this feature???

Yeah, I've reviewed again the whole thread and realized that @samitnuk proposed the same at the beginning 😄 .