nopCommerce 4.40 - Bug fixes and improvements

foxnetsoft wrote:

Done! Here's the commit

Hi Andrei,

I am not sure you have seen my reply here:
https://www.nopcommerce.com/en/boards/topic/90152/nopcommerce-440-bug-fixes-and-improvements/page/4#281315

Moreover, these methods used to be public in the previous versions of nopCommerce so probably many nopCommerce developers are already using them in their custom plugins.
I will appreciate it if you reconsider this!

Thank you!
Boyko

Hi, Boyko.

I would like to understand what the problem is to build the entire model and take only the necessary data from it. We specially removed methods from the interfaces that are not used outside of their implementations, but we leaving the possibility of using them in the inherited classes. We see no reason to keep such methods in interfaces (technically, they should not have been there initially), and making them just public is also not an option, since the final implementation of the factory can be changed by any plugin.

Nop-Templates.com wrote:

Hi Sergei,

Thank you for your reply and for the very good questions!

Sergei-k wrote:

There are many cases where you need only part of the model (inner models) and building the whole model in most cases is a very heavy operation as a model could consist of several other models. So doing so will slow the plugins down. In other cases, even building the whole model is not working for us as we need to have some more granular control of the inner models.
Let me give you a real-world example from our code so that you can get an idea of what we are after.
In our Ajax Cart plugin, we show a popup with information for the currently added product i.e show its picture, etc.
To do so we need to call this method PrepareCartItemPictureModelAsync with the picture size set in the administration of the plugin (we don't want to use the default picture size used for the mini-shopping cart items).
So in this case even if we build the whole model, which by the way is completely unnecessary as we don't need to calculate discounts, taxes, etc. we still could not reuse the picture model as the picture is with the wrong size. So we still need to call PrepareCartItemPictureModelAsync, which is now protected and we can't use it anymore. Previously we could simply call the method directly and reuse the logic.

Sergei-k wrote:

Yes, these methods are not used in the nopCommerce codebase itself but it doesn't mean that they should not be used by plugin developers. In reality, these methods are quite useful for plugin developers as they can use them in their plugins.
If you make these methods public and part of the interfaces of course then all plugin developers will be able to reuse the nopCommerce logic as much as possible rather than "copy" it.
At the moment we have to create our own factories and inherit from your factories in order to reuse the code. I think that this is completely unnecessary and making the methods public will only make the lives of the plugin developers a bit easier.
From an extensibility point of view, I think you should try to urge plugin developers to reuse the existing code as much as possible. After going through 20+ nopCommerce upgrades already I know very well that the very best practice for nopCommerce plugin developers is to reuse as much code as possible from the one already there. And not being able to do so is very very frustrating.

Sergei-k wrote:

Technically you are right but in reality, these Factories are there to be reused, right? (at least from the perspective of extending nopCommerce via plugins). So keeping them closed is of absolutely no use to anyone extending nopCommerce.

Sergei-k wrote:

I don't quite understand that.
Now everyone could create their own factory and change the implementation.
Actually, if someone changes the implementation of the factory and we don't use the factory methods (since they are protected) then our plugins will not be "consistent" with the rest of nopCommerce as they use a "copy" of the original logic rather than reusing the factory logic.
So again I think it is even better to keep all the methods that build models to be part of the interface and let everyone consume them in their plugins (even though you may not use these methods in your codebase).

I hope you will see our point of view now and if you have any other questions I will be happy to discuss them!

Thank you again for your time!
Boyko

Hi Boyko.

Thanks for the detailed analysis of the issue, I reopened the ticket https://github.com/nopSolutions/nopCommerce/issues/5099 and will return all (or almost all) methods back to interfaces

Hi guys,

There are some hard-coded styles here.
Could you please move them to the css files so that we can style them in the themes without overriding these views?

Thanks,
Boyko

Nop-Templates.com wrote:

Thanks, Boyko! Sure - https://github.com/nopSolutions/nopCommerce/issues/5503

Hi guys,

The UpdateAsync method does not take into account the publishEvent parameter and no event is being published.

Thanks,
Boyko

When you visit a category without direct products i.e Computers.

This message is shown under the images of the subcategories:

No products were found that matched your criteria.

I believe this is needed only after filtration.

p.s: Used latest GitHub source code

Thanks,
Boyko

Nop-Templates.com wrote:
Thanks, Boyko. Here is work item

Nop-Templates.com wrote:
And here is the second one