nopCommerce 4.30 - Bug fixes and improvements

1 month ago
Done. Please see this commit for more details

a.m. wrote:
Currently it is not possible to uninstall a plugin that has tables with foreign keys....
Thanks a lot, Boyko! We'll check it - https://github.com/nopSolutions/nopCommerce/issues/4372
1 month ago
Sergei-k wrote:
Done. Please see this commit for more details]


Many thanks Sergei!

I just tested it and all seems to be working fine now.

Have a great weekend!

Thanks,
Boyko
1 month ago
Sergei-k wrote:
We fixed the problem, with the naming of tables, please see this ticket for more details.

In general, you need to implement the INameCompatibility interface(see BaseNameCompatibility class for example)


Hi Sergei,

One thing that I noticed is that the new interface is not used when creating the names of the primary keys and indexes for the tables.
For example if your entity is Slider and you specify the table name to be SS_Slider.
Then the primary key will be "PK_Slider" instead of "PK_SS_Slider" and the same for the Index.
I am not sure if this is really needed but for the sake of consistency I think it should also use the specified table names in INameCompatibility instead of the type of the entity.

p.s: Foreign keys are working fine and they use the TableNames properly.

Thanks,
Boyko
1 month ago
Thanks again, Boyko. We'll check it soon - https://github.com/nopSolutions/nopCommerce/issues/4378
1 month ago
Nop-Templates.com wrote:

...One thing that I noticed is that the new interface is not used when creating the names of the primary keys and indexes for the tables...


Hi Boyko. I understood about the PK, I’ll fix it. But I didn’t understand what indexes we are talking about since I checked the indexes seem to be created with the correct names. Could you place describe how you create the indexes
1 month ago
Sergei-k wrote:

...One thing that I noticed is that the new interface is not used when creating the names of the primary keys and indexes for the tables...


Hi Boyko. I understood about the PK, I’ll fix it. But I didn’t understand what indexes we are talking about since I checked the indexes seem to be created with the correct names. Could you place describe how you create the indexes


Hi Sergei,

I was talking about the Index that is created as part of the primary key.
When you open the table in MS SQL Server Management Studio you have several folders i.e Columns, Keys, Constraints etc. so this is where I saw the Indexes folder and it contains this index name PK_Slider (Clustered).
I guess when you correct the name of the primary key then the index will automatically have the same name as the primary key so I don't think you need to do anything extra about it.

Many thanks!
Boyko
1 month ago
Nop-Templates.com wrote:

I was talking about the Index that is created as part of the primary key.


I made changes to PK naming, details in this commit. Thanks for the help.
1 month ago
Sergei-k wrote:

I was talking about the Index that is created as part of the primary key.


I made changes to PK naming, details in this commit. Thanks for the help.


Hi Sergei,

I just tested it and I can confirm that both the PK and the Index now use the specified name for the table.

Many thanks again!
Boyko
1 month ago
This is something I have observed over the course of the release from 4.2 to this new version: A lot of breaking changes or new additional features are not really documented well on the docs and thus making harder for plugin developers to explorer them. Particularly for this version, for example, the new migration feature is great, however, I think it is too much of a blackbox. The reason I said that is before Nop switched to FluentMigrator, I used that in my plugin to have a better control for DB migrations. Now Nop included it by default and that is great, but that would mean I have to refactor my code so that it will not conflict with the default behavior. And there is no documentation on how to avoid those pitfalls. To get better idea on the new migration, I would have to dig into the Nop codebase and read it which could take more time than reading a good documentation.

Another example is that the way the mapping is handled in this version. I get that it eases the complexity but if I want to customize the table name or column name, I would have to use the new INameCompatibility interface. It would be better to provide an alternative way similar to the other approach (customize mapping per individual class).

Also, back to the migration stuff, one suggestion that I think it would be better is to provide a way to delete table quickly like the previous DropPluginTable and use it during the uninstall of the plugin. The current logic of uninstall plugin is heavily relying on the Down migration and that could cause an issue if there is something wrong during those migrations. Since a plugin is getting deleted, would that be better just to execute raw sql to delete the plugin tables to completely remove any extra stuff as well as skipping some unnecessary migration downgrade. Moreover, I do think each plugin need to have it own migration version table so that it does not pollute the main migration version table. Bundle plugins can be in the main but for any other, it is better to separate them out. I did that with my own and it's easy to see which one having issue or not.
1 month ago
Hi guys,

There is a problem when saving the ACL.
You can easily reproduce it following these steps:

1. Open ACL page in administration
2. Check "Admin area. Manage plugins" for the Vendors role.
3. Click "Save"
4. After the page is saved, click "Save" again or enable another permission per role and click "Save"

You should get this error:

SqlException: Violation of PRIMARY KEY constraint 'PK_PermissionRecord_Role_Mapping'. Cannot insert duplicate key in object 'dbo.PermissionRecord_Role_Mapping'. The duplicate key value is (40, 5). The statement has been terminated.

Thanks,
Boyko