wintercms / wn-builder-plugin

GUI for building plugins in Winter CMS
MIT License
34 stars 12 forks source link

Editing plugin details removes the 'replaces' key from plugin.yaml #10

Closed ericp-mrel closed 2 years ago

ericp-mrel commented 2 years ago

If I have the following plugin.yaml file:

# plugin.yaml

plugin:
    name: 'acme.blog::lang.plugin.name'
    description: 'Plugin Description'
    author: 'Author Name'
    icon: wn-icon-chain
    homepage: 'https://example.com'
    replaces:
        Acme.Blog: '<3.0.0'

And I edit the plugin details by clicking on the plugin gear icon and save the changes, the replaces definition is removed from the yaml and results in the following:

# plugin.yaml

plugin:
    name: 'acme.blog::lang.plugin.name'
    description: 'Plugin Description'
    author: 'Author Name'
    icon: wn-icon-chain
    homepage: 'https://example.com'
arvislacis commented 2 years ago

I think this is because Builder plugin allows you to only edit name, description, author, icon and homepage fields when you open plugin configuration pop-up (click on gear icon). And only these fields are then saved back to YAML configuration file after save.

I personally don't see this as a big issue because you probably added replaces section externally.

One possible solution may be to add replaces repeater or datatable field into configuration pop-up and logics so it gets properly loaded and saved.

ericp-mrel commented 2 years ago

I don't think this issue is that big of a deal and it's not very often that I need to edit the plugin details via that menu. But it'd be nice if this plugin would account for the new replaces key and would keep it in the output when re-writing the file. Even better would be to add a field to allow adding replacements via this plugin, but that would be just a nice to have.

It seems like it's a pretty easy fix to have it maintain the replaces dictionary upon writing the plugin.yaml file. I can send a PR if that's the route the maintainers would like to go rather than adding UI functionality (at least for now).

arvislacis commented 2 years ago

@ericp-mrel May I also ask where did you find out that you can use replaces in plugin.yaml configuration? As far as I quickly searched then you can use it under pluginDetails() function - https://wintercms.com/docs/plugin/registration#plugin-replace - but I am not sure about plugin.yaml. Does it even work when used from YAML file?

ericp-mrel commented 2 years ago

@arvislacis If you look at the PluginBase.php class, you can see the base pluginDetails() method will parse the data from the plugin.yaml file if it exists and returns the data under the plugin: key as an array. See https://github.com/wintercms/winter/blob/develop/modules/system/classes/PluginBase.php#L57-L73

Otherwise, if you don't have/use the plugin.yaml file you would override the pluginDetails() method in your Plugin's Plugin.php file and define the array there.

And yes, the replaces key does work in the YAML file, I now have two plugins defining the replaces in the plugin.yaml file and is working just fine. After all, the yaml file just gets parsed into a PHP array anyways.

Hope this makes sense!

LukeTowers commented 2 years ago

@ericp-mrel give #11 a test please

bennothommo commented 2 years ago

@LukeTowers @arvislacis don't you guys mean #11? 😛

LukeTowers commented 2 years ago

Whoops!

ericp-mrel commented 2 years ago

@LukeTowers It's working for me, even tested it with multiple replaces items

Example of what I've tested.

replaces:
  Acme.Plugin: '<1.0.0'
  Winter.Plugin: <5.5.5