Django-oscar: Allowing OSCAR_HIDDEN_FEATURES apps to be excluded from URLConf

Created on 20 Jun 2020  路  3Comments  路  Source: django-oscar/django-oscar

I have config

OSCAR_HIDDEN_FEATURES = ['reviews', 'voucher', 'wishlists']

Voucher is very deeply coupled with basket internals, so couldnt possibly be removed entirely.

I can appreciate that "hidden" means the apps oscar.apps.wishlist & oscar.apps.catalogue.reviews still need to be in INSTALLED_APPS, however I believe they should be excluded from URL paths. Exposing these features on the site isnt really 'hidden' in my perspective. I am assuming that those views will be inoperative, to some extent, but they really should not be exposed at all. Leaving them on the site, but hidden, is a concern that they might be used by hackers, but they are out of sight, out of mind, from the site admins.

CustomerConfig depends on wishlists, and DashboardConfig tries to load reviews_dashboard. BasketConfig doesnt require voucher, but deeper down there were some reverse which require the voucher views.

I have hacked those so they dont emit url patterns for those apps, if they are in OSCAR_HIDDEN_FEATURES. Is that a direction Oscar wants to go in? If so, I am happy to contribute that.

If there is interest, I would be happy to take it a bit further to allow excluding reviews or wishlists apps from INSTALLED_APPS, however I suspect this will be harder to build tests for. This is less important afaics - my primary goal was removing those apps from the URLConf, but removing them from INSTALLED_APPS was a way to validate that there wasnt any remaining unknown hooks into those hidden features. we are running without reviews or wishlists in INSTALLED_APPS - vouchers is not worth the pain to remove, and likely we would want its functionality in the near future anyway.

Most helpful comment

A couple of thoughts on this:

  1. In the long run we need to try to decouple Oscar's apps as much as possible, and make them pluggable. reviews and wishlists could without too much effort be made completely standalone and optional. voucher is trickier but there are still unnecessary interdependencies which could be improved.

  2. I would like to drop OSCAR_HIDDEN_FEATURES entirely and instead move to a pluggable app architecture: to disable you reviews you would simply remove it app from INSTALLED_APPS. In templates, iffeature would then just become a wrapper for if apps.is_installed() (or replaced by some equivalent).

The reviews and/or wishlist apps would I think be a good test case to validate that this approach is viable, so if you have time to work on it that would be great.

All 3 comments

A couple of thoughts on this:

  1. In the long run we need to try to decouple Oscar's apps as much as possible, and make them pluggable. reviews and wishlists could without too much effort be made completely standalone and optional. voucher is trickier but there are still unnecessary interdependencies which could be improved.

  2. I would like to drop OSCAR_HIDDEN_FEATURES entirely and instead move to a pluggable app architecture: to disable you reviews you would simply remove it app from INSTALLED_APPS. In templates, iffeature would then just become a wrapper for if apps.is_installed() (or replaced by some equivalent).

The reviews and/or wishlist apps would I think be a good test case to validate that this approach is viable, so if you have time to work on it that would be great.

voucher is trickier but there are still unnecessary interdependencies which could be improved.

Basket model has m2m relation to voucher model, so not sure how to decouple this. My only particular idea is to implement mixin class and include voucher-related fields and methods only if voucher application is enable.

@solarissmoke please share your thoughts/ideas about that.

Basket model has m2m relation to voucher model.

I have not looked into it closely, but in theory it should be possible to move the field into the Voucher model? Since it is an M2M it would be the same relationship either way.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rollcat picture rollcat  路  3Comments

aravind12345 picture aravind12345  路  6Comments

jayvdb picture jayvdb  路  7Comments

vividvilla picture vividvilla  路  4Comments

maerteijn picture maerteijn  路  5Comments