Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#85 2011-12-17 23:13:24

maniqui
Member
From: Buenos Aires, Argentina
Registered: 2004-10-10
Posts: 3,070
Website

Re: adi_matrix – Multi-article update tabs

In the meanwhile, I think I’ll temporarily hack my copy of adi_matrix and just put:

$glz_cfs_installed = 1;

:)


La música ideas portará y siempre continuará

TXP Builders – finely-crafted code, design and txp

Offline

#86 2011-12-18 00:01:29

maniqui
Member
From: Buenos Aires, Argentina
Registered: 2004-10-10
Posts: 3,070
Website

Re: adi_matrix – Multi-article update tabs

maniqui wrote:

Adi, could it be possible for you to add another test to check if glz_c_f is present?
I’m proposing an OR test using function_exists, but may there be a more bullet-proof of testing the presence of a particular plugin (not relying on the DB and not relying on some particular function)?

I’m thinking of something like:

$glz_cfs_installed = safe_row("version","txp_plugin","status = 1 AND name='glz_custom_fields'") || function_exists("some_glz_c_f_function");

Mmmmm, I’ve tested this but it won’t work unless I make TXP load first the glz_custom_field plugin, for example, by renaming glz_custom_fields.php to something like aaa_custom_fields.php, so it gets loaded before adi_matrix.php.
It works, so certainly, that could be a solution, but if I could avoid it, I’ll be happier :)

Another similar solution could be to test for the presence of a particular glz_custom_fields preference, although the existing ones (in txp_prefs table) have some pretty generic names, and again, doesn’t seem a flawless solution.

So… Adi, to avoid hacking your plugin, this is another solution I can think of: could you consider adding an option (a checkbox to be manually checked) to mark the glz_custom_fields plugin as installed?
In other words, a way to “manually” tell adi_matrix:
“Yes, believe me, glz_custom_fields is installed, don’t worry if you can’t find it at the txp_plugin table. So, adi_matrix, do your extra glz_c_f magic.”


La música ideas portará y siempre continuará

TXP Builders – finely-crafted code, design and txp

Offline

#87 2011-12-18 00:18:06

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: adi_matrix – Multi-article update tabs

maniqui wrote:

I’m proposing an OR test using function_exists, but may there be a more bullet-proof of testing the presence of a particular plugin (not relying on the DB and not relying on some particular function)?

As you say, relaying on the database row is a bad practice. To check whether particular plugin is installed (and to make sure the code is usable when needed), one can simply use Textpattern’s require_plugin(string $name). I.e.

$glz_cfs_installed = require_plugin('glz_custom_fields');

The function will return (bool) false when required plugin is missing, and triggers a (debugging) notice too. The function will make sure that either;

  • Required plugin exists in the database, and is active. If the plugin is active (or force is set true) and it hasn’t been loaded, it will load the plugin. This is to make sure that the plugin code is actually initialized and available when needed. If force is set true, plugin’s status won’t matter and the plugin is loaded despite it being disabled.
  • Required plugin is in the plugin cache directory. The function will check that file named as $name.php (i.e. glz_custom_fields.php) exists in the directory. If it does, it includes the plugin’s code (file) if it wasn’t already included.

Edit. that $force parameter is from load_plugin().

Last edited by Gocom (2011-12-18 08:09:21)

Offline

#88 2011-12-18 00:31:26

maniqui
Member
From: Buenos Aires, Argentina
Registered: 2004-10-10
Posts: 3,070
Website

Re: adi_matrix – Multi-article update tabs

Gocom, thanks once again for sharing your deep understanding of TXP.

I can confirm that:

$glz_cfs_installed = require_plugin('glz_custom_fields');

works as expected.


La música ideas portará y siempre continuará

TXP Builders – finely-crafted code, design and txp

Offline

#89 2011-12-18 06:46:52

gomedia
Plugin Author
Registered: 2008-06-01
Posts: 1,373

Re: adi_matrix – Multi-article update tabs

Gocom wrote:

To check whether particular plugin is installed (and to make sure the code is usable when needed), one can simply use Textpattern’s require_plugin(string $name, bool $force = FALSE)

adi_matrix doesn’t require glz_custom_fields to be installed. It only needs to detect if it is installed & then modify it’s behaviour accordingly. And I don’t want error messages if it’s not installed.

If the plugin is active (or force is set true) and it hasn’t been loaded, it (i.e. require_plugin) will load the plugin.

adi_matrix is definitely not going to load any other plugin if the user hasn’t done it themselves already.

As you say, relying on the database row is a bad practice.

Are there scenarios where a plugin exists in the txp_plugin database table, with a status set to “1”, and it’s not actually loaded & active?

Textpattern’s require_plugin(string $name, bool $force = FALSE)

I couldn’t find a $force option to this function in TXP 4.4.1 – is it in an upcoming release?

maniquiWRT the scenario where glz_custom_fields is loaded from the plugin cache – I’m happy to look into other ways of detecting it.

Offline

#90 2011-12-18 08:04:27

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: adi_matrix – Multi-article update tabs

gomedia wrote:

I couldn’t find a $force option to this function in TXP 4.4.1 – is it in an upcoming release?

Nope, sorry my mistake. It’s not parameter of the function at all. It’s from load_plugin() which is what require_plugin() uses. Both are the same thing (aliases) expect that require_plugin additionally returns notices.

adi_matrix doesn’t require glz_custom_fields to be installed. It only needs to detect if it is installed & then modify it’s behaviour accordingly. And I don’t want error messages if it’s not installed.

If you don’t want notices, you can use load_plugin().

$glz_cfs_installed = load_plugin('glz_custom_fields');

Same thing as with require_plugin(); returns true if the plugin exists and is active.

adi_matrix is definitely not going to load any other plugin if the user hasn’t done it themselves already.

require_plugin(), and load_plugin(), will load nothing, or affect loading, if executed on a callback event. After the initial plugin loading is done by Textpattern, you can safely use both load_plugin() and require_plugin() without them doing anything else than checking.

Last edited by Gocom (2011-12-18 08:22:37)

Offline

#91 2011-12-18 12:30:28

redbot
Plugin Author
Registered: 2006-02-14
Posts: 1,410

Re: adi_matrix – Multi-article update tabs

Hi Adi,
I think I found a bug with checkboxes generated by glz_custom_field:
checking works ok but unchecking doesn’t seem to (preferences are not saved).
Everything is ok with radio buttons.

edit:
More specifically,
if you have only one option checked you won’t be able to uncheck it (when you hit “save” changes are not saved).
Note that unchecking works ok if you have more than one option checked.

Last edited by redbot (2011-12-18 16:33:53)

Offline

#92 2011-12-18 17:08:52

mrdale
Member
From: Walla Walla
Registered: 2004-11-19
Posts: 2,215
Website

Re: adi_matrix – Multi-article update tabs

Here’s an usability idea for this incredibly useful plugin.

I often end up with a lot more than 10 custom fields to display on the x axis. This requires scrolling to the right and obscuring the the article title. I don’t know how you would do this (a second table with just titles to the left) but presenting a frozen pane with titles on the left would be really helpful.

Awesome plugin BTW, probably one of the best UI plugins ever for TXP.

Offline

#93 2011-12-18 18:37:55

masa
Member
From: Asturias, Spain
Registered: 2005-11-25
Posts: 1,091

Re: adi_matrix – Multi-article update tabs

mrdale wrote:

I don’t know how you would do this (a second table with just titles to the left) but presenting a frozen pane with titles on the left would be really helpful.

That gets my vote, too.

Awesome plugin BTW, probably one of the best UI plugins ever for TXP.

I totally agree!

Offline

#94 2011-12-19 01:22:28

gomedia
Plugin Author
Registered: 2008-06-01
Posts: 1,373

Re: adi_matrix – Multi-article update tabs

redbot wrote:

I think I found a bug with checkboxes generated by glz_custom_field checking works ok but unchecking doesn’t seem to (preferences are not saved).

I’ve sent you a beta version to try …

Offline

#95 2011-12-19 01:23:53

gomedia
Plugin Author
Registered: 2008-06-01
Posts: 1,373

Re: adi_matrix – Multi-article update tabs

mrdale wrote:

… presenting a frozen pane with titles on the left would be really helpful.

I like that idea, very much. I’ll put it on the list.

Offline

#96 2011-12-19 02:18:50

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: adi_matrix – Multi-article update tabs

I took quick look at adi_matrixe’s code and there seem to be some conflicts and issues;

  • There are some SQL injection possibilities. All user provided data that is passed to SQL queries should be escaped using doSlash(), i.e. in adi_matrix_update_article(). There are some that are not.
  • The code at the top of the plugin (wrapped in the conditional tags) is in a scope it shouldn’t be. All that code should be wrapped in a function to avoid conflicts.
  • if (isset($prefs['plugin_cache_dir'])) is always set. It can be empty, but the variable is always there. To get the performance benefit you are probably trying to get, you could change that to if(empty($prefs['plugin_cache_dir'])).
  • Global $glz_cfs_installed should be prefixed to avoid potential conflicts.
  • To avoid potential collisions and improve functionality, adi_matrix_get_articles()’s category (any, none etc) ranges selection could be easily changed so they don’t conflict with real categories — same with usernames (author). This can be easily done by using selector values that are longer than the database field’s maximum length. For author this could be done by using empty value for example.

With following:

if (!strpos($prefs['plugin_cache_dir'],'adi') === FALSE) 

You probably mean:

if (strpos($prefs['plugin_cache_dir'],'adi') !== FALSE)

Last edited by Gocom (2011-12-19 02:29:25)

Offline

Board footer

Powered by FluxBB