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.
A couple of thoughts on this:
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.
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.
voucheris 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.
Most helpful comment
A couple of thoughts on this:
In the long run we need to try to decouple Oscar's apps as much as possible, and make them pluggable.
reviewsandwishlistscould without too much effort be made completely standalone and optional.voucheris trickier but there are still unnecessary interdependencies which could be improved.I would like to drop
OSCAR_HIDDEN_FEATURESentirely and instead move to a pluggable app architecture: to disable you reviews you would simply remove it app fromINSTALLED_APPS. In templates,iffeaturewould then just become a wrapper forif 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.