nopSolutions / nopCommerce

ASP.NET Core eCommerce software. nopCommerce is a free and open-source shopping cart.
https://www.nopcommerce.com
Other
9.3k stars 5.32k forks source link

Do not store LimitedToStores / LimitedToRoles in plugin.json because of CI/CD #6636

Open Manuel-Innovapps opened 1 year ago

Manuel-Innovapps commented 1 year ago

nopCommerce version: any

Steps to reproduce the problem:

  1. As of today, we have setup CI/CD Pipelines for nearly all of our customers, where we deploy nopCommerce from Bitbucket Pipelines to their hosting Server (Mostly Azure)
  2. During deployment the plugin.json for every plugin is overridden, which would require us to save LimitedToStores / LimitedToRoles inside our repo, which doesn't make a lot of sense.
  3. Also if the customer decides to make a change to LimitedToStores / LimitedToRoles during runtime, nobody will notice, and we would override the file with the old information during the next deployment.

As CI/CD pipelines become much more common these days, we think that we can't be the only ones having this problem.

Also we haven't figured out a viable workaround for this, expect to override the appropriate PluginManager, which cause some work.

skoshelev commented 1 year ago

It seems to me easier to set up the СD in such a way that it does not overwrite the configuration files. Historically, we don't store any information about plugins in the database and allow users to change plugins through the file system, otherwise the plugin system would be very inflexible. I do not see a way to transfer this functionality without global changes.

Manuel-Innovapps commented 1 year ago

But not deploying the plugin.json would bring other problems. What about if we update the plugin version locally to add a new migration? If we don't overrwrite the plugin.json, the migration would not be executed.

I see your point, but as you suggest, I think this would require global changes.

skoshelev commented 1 year ago

Well, that's a good clarification. I was thinking that we could consider a fairly simple migration of the LimitedToStores / LimitedToRoles properties to the root plugins.json file. I guess it won't be too hard and will solve the CI/CD problem

Manuel-Innovapps commented 1 year ago

Yep, I think that should work out. We initially also tried to deploy the plugins.json, but we have stopped that trough all customers, because of other issues.

So deploying the plugins.json is not a good idea in itself, so I think your idea should work out!

Manuel-Innovapps commented 1 year ago

Is there any update on this?

We also experienced this with Widget-Display order, so the problem is bigger than initially expected

Manuel-Innovapps commented 1 year ago

@AndreiMaz @skoshelev Are there any plans to incorporate this into the next version?

If not we will be required to find a custom solution for our customers for this general problem! Thanks for your opinion!

AndreiMaz commented 5 months ago

@Manuel-Innovapps After some consideration we've decided not to implement this functionality out of the box and leave it for customization

Manuel-Innovapps commented 3 months ago

@AndreiMaz @skoshelev This just came up again today, and I was honestly shocked that this ticket was closed. I assume every of the StoreOwners that has the source code will experience this problem. So without having any statistics, I would assume that this problem should exist with at least half of your customer base.

Here are two points my colleague brought up:

Could you please explain why this was marked for customization? This is a core problem that can be easiest solved in the core. All other solutions will require some hacky workarounds and probably requires modifying the core solution, which will block easy upgrade paths in the future.

AndreiMaz commented 3 months ago

@Manuel-Innovapps Please see the first reply and suggestion of Sergei at https://github.com/nopSolutions/nopCommerce/issues/6636#issuecomment-1457676320. Unfortunately, we do not plan to store this configuration information in the database

Manuel-Innovapps commented 3 months ago

@AndreiMaz As discussed later on this issue, this solution is not viable at all, please see also the arguments I brought up two days ago. As discussed in this issue the goal is not to store this information in the database, but rather in App_Data/plugins.json. That would solve all the problems mentioned here!

skoshelev commented 3 months ago

Hi @Manuel-Innovapps.

I understand that transferring this data to the App_Data/plugins.json file can be useful, but it is not so easy to do.

I can’t understand why you need to change SupportedVersions field when releasing minor releases. This field must store only the major component.

The only problem is updating the plugin or rather its version, but I think this can be abandoned within CI/CD and store the real version inside the plugin itself and simply not use the default update mechanism

Manuel-Innovapps commented 3 months ago

As you said, it's up to you to decide on what to implement and what not.

Your suggestion will make every nopCommerce partner that is hosting shops for their customers re-invent the wheel for managing updates, although a great way with good migrations already exists. It would only require some minor changes in the core, and not hundreds of custom implementations by each partner for their respective customers.

skoshelev commented 3 months ago

Hi @Manuel-Innovapps.

I understand that for your case with the distribution of the final product via CI/CD this is a problem.

I don't know how many other developers use this approach, especially since the nopCommerce development cycle does not allow for backward compatibility from version to version. But if we should rework the plugin system for compatibility with CI/CD, we need to remove almost all fields from the plugin.json file. For example, FriendlyName can be localized by the user for his language, as well as the Description field.

Another question when deleting a plugin is whether it is necessary to delete data from the global file if the user manually deletes the plugin from the file system. How can a user on your system delete a plugin if CI/CD forcibly returns all files to him including plugin.json?

Manuel-Innovapps commented 3 months ago

Our main case are our internal plugins - developed by us for our customer. They usually don't want to delete those plugins. That's also the main source of the problem - our internal plugins.

As for 3rd party plugins, they are only able to uninstall the plugins, not deleting them.

Some customers will notify us - and we will delete the plugin from the source code.