Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using plugin['title'] for logical checks is utterly incorrect #73

Open
imfx77 opened this issue Feb 16, 2024 · 2 comments
Open

using plugin['title'] for logical checks is utterly incorrect #73

imfx77 opened this issue Feb 16, 2024 · 2 comments

Comments

@imfx77
Copy link

imfx77 commented Feb 16, 2024

I have just placed the following PR to Kanboard:
a fix for the plugin directory

In PluginManager you have most probably borrowed that ill-considered code, and did build upon it, and did multiply it for various purposes. This makes it a bit more tangled at your side 🤔
They may choose to merge or to ignore my PR in Kanboard, but it doesn't concern you either way since you override the entire behavior.

So, have a look at the PR changes and consider applying them analogically to PluginManager/Template/plugin/directory.php.

<?php foreach ($available_plugins as $plugin): ?>

However, bear in mind that the $available_plugins is an associative array of the form (key => value), the "key" being the plugin name and the "value" an array of the plugin properties.
And you have an extra problem because you effectively destroy the keys when you sort the array here:

<?php usort($available_plugins, fn ($a, $b) => strtolower($a['title']) <=> strtolower($b['title'])); ?>

@aljawaid
Copy link
Owner

Hi thanks for that.

I will leave it as it is for now and will monitor it when the next release of KB is out.

The core code is messed up but works in KB and I know the collabs already worked very hard and sometimes pulling their hair out maybe trying to get the pkugin side to work. Obviously you seen the state of the plugin director without thia plugin lol.

Let's see how it goes.

@imfx77
Copy link
Author

imfx77 commented Feb 17, 2024

I will leave it as it is for now and will monitor it when the next release of KB is out.

My PR was merged this morning in the main KB branch. I don't think the KB release will impact your plugin in any way. But if you prefer to wait for it, so be it.

The core code is messed up but works in KB

Nope, even though nothing breaks in a major way, it doesn't work properly either. The issue in the plugin directory both for core KB code and for PluginManager is the same - namely, assuming that a plugin title and a plugin name are equivalent. Which is not the case, and I can't imagine why one would assume that. If you go and check the plugins.json more than 1/3 of the plugins have titles that don't match their name. And when the plugin directory code looks up by title for those plugins no match is found in the $installed_plugins, and they simply appear as not installed. Need to check for updates and manage them externally and manually. Which was the purpose of the plugin directory in the first place I believe ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants