nopCommerce 4.30 - Bug fixes and improvements

Sergei-k wrote:

Thank you pointing this out that the CacheKey is designed to be Property and that a new one should be created every time! That explains everything!!!

Actually I didn't noticed this during the upgrade and in one of our plugins I made the CacheKey to be fields:

public static readonly CacheKey CacheKey = new CacheKey(...);

This explains why I got this problem.
Now when I changed the filed to be a property I no longer have this problem.

Please note that the problem still remains when you don't use a property and I am sure many developers will not notice that the CacheKey should be a property and a new one should be created every time (just like me) and they will end up with some very very strange problems with the caching. Since there is nothing to prevent us from making the CacheKey to be a field I think this is a design problem.
I personally thought that FillCacheKey will always create a new CacheKey rather than return this and it looked a bit strange to me but I was in a hurry and didn't pay too much attention to how you have implemented the CacheKeys exactly in your code.

Thank you for your time!
Boyko

Hi guys,

We really like the new Update plugin functionality as it will allow us to add new features that require db changes without waiting for a new nopCommerce release and to rely on the Upgrade scripts.

We would like to put some of our backward compatibility logic of our plugins into the Update method of the plugin itself. Also we would like to put some of the "upgrade" logic into a separate Migration rather than into an upgrade script.
But currently the Update plugin logic as well as the check for the Migrations is executed only after the plugin has an older version set, which is not the case when upgrading from previous versions of nopCommerce where version is missing in the plugins.json file.

Could you please perform the same logic as if the version is updated when there is no version specified i.e when upgrading from 4.20 and using the old format of plugin.json?

This will allow us to add only the base schema migration in the Upgrade script and any other logic will go into the migrations from now on. Furthermore we plan to use Migrations to introduce new features in the plugins in 4.30 and thus we will be sure that these migration will be executed if someone upgrades their websites from 4.20 (or older versions) after we introduce these changes.

Thank you for looking at this!
Boyko

Hi,

Hopefully a simple question - we're just about to get started with nopCommerce and so have downloaded the 4.30 beta version to start developing a plugin. The current documentation for 4.20 includes (for example) info on adding tables to the existing database - given the removal of EF, I suspect that this and possibly other tutorials will need to be updated?

If so, could you please let me know when you're planning to update so we can get stuck into development?

Thanks,

Matt

Hi, Boyko.
Initially, we didn't add such behavior in order to avoid problems in the future. But thanks to your description, I realized that problems can arise anyway. Now the plugin update process runs and when the plugins.json file format changes. But instead of the old version, the null value will be passed. You should also consider that migrations that initiate the initial structure of tables should be marked with the SkipMigrationOnUpdate attribute so that they would not be performed when updating the plugin. You may see details in this commit.

Nop-Templates.com wrote:

Sergei-k wrote:

Hi Sergei,

Thank you for doing this! It will be quite handy!

I am interested to know your ideas about the the migrations and the upgrade scripts and specifically how exactly you envision to use them in the future i.e future upgrades of nopCommere and official plugins.

Do you plan to still use Upgrade Scripts for future upgrades or you will add all the changes you do into new Migrations or it will be a mix of both?
If it will be a mix then what you plan to keep in the Upgrade scripts and what in the Migrations.

I could only guess what your ideas are but I would like to see your point of view on this.

Thanks,
Boyko

Hello, Boyko.

We plan to transfer everything to migration, and fully automate the update process. But most likely only for versions older than 4.30, that is, the upgrade process to 4.30 inclusive can remain the way it is now.

Nop-Templates.com wrote:

Hello nopCommerce.

We found some issues in the html markup for the new "product reviews avatar" functionality. The problem is in the duplicated html containers, for some reason the "review-item-head" element and the "review-title" element are cloned and the clones are used in a non-semantic way, for example the cloned "review-title" is now containing the avatar picture. This is not SEO friendly and besides, using the same  class names for containers that are not identical will definitely lead to css conflicts.

We edited the html markup for you and fixed the css styling according to the changes, and created a pull request:

https://github.com/nopSolutions/nopCommerce/pull/4608

Please consider to merge this in the final 4.3 version. Thank you :)

Best regards
/ Hristo

Sergei-k wrote:

Hi Sergei-k,

Great! This is what I though you would do but it is nice to hear it from you :)

Thanks,
Boyko

Hi guys,

It seems like you limit the Migrations that we could use to be only of type AutoReverseMigration.
See the code here:
https://github.com/nopSolutions/nopCommerce/blob/5fa538d75e9f9d234c7a7b40af280ba8fd4eac75/src/Libraries/Nop.Data/NopDbStartup.cs#L27

Could you please allow all types/assemblies that implement IMigration to be searched?

We want to use some migrations that won't be AutoReverseMigration and we inherit directly from the Migration class (actually we have a base class that inherits from the Migration class where we put some common logic) but now the migrations for these plugins are skipped and never executed by nopCommerce.

Thank you in advance!
Boyko

Hi, Boyko.
I understood the problem and will make changes, only we will use the MigrationBase class to search and not the IMigration interface (I hope this level of abstraction will be enough). But even now any migration will be performed, if its assembly is loaded, the main thing is that there would be at least one AutoReverseMigration migration in the assembly, then all other migrations will also be performed.

Nop-Templates.com wrote: