Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#21 2012-07-26 17:13:43

Gocom
Plugin Author
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: Plugin filtering for admin list pages

I wouldn’t put the callback event to the DB functions. It seems the wrong place to me. I would add it to the corresponding locations where list filtering is needed. Application and its callbacks have no place in the DB abstraction layer.

In any case, adding the callback event to safe function doesn’t even shorten the code significantly. No new possibilities are introduced, since the option to use callbacks_events in those contexts is always there nonetheless what DB wrapper does. The panel specific code still needs alterations where the callback parameter is supplied.

As far as performance and security goes, this would have no effect to either. An extra parameter and boolean operator don’t do nor use shit, and security hazards — no.

Offline

#22 2012-07-26 19:25:07

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,631
Website

Re: Plugin filtering for admin list pages

OK, thanks for the input guys. One more question: what’s the best name for the event? As mentioned if the event is simply set up like this:

$criteria .= callback_event('image', 'criteria', 0, $criteria);

then any plugin that hooks into that panel’s event (alone) will run when the ‘criteria’ step is hit. That’s pretty much a guarantee that it’ll break the output. If we could rely on there being a fixed step then it wouldn’t be so bad, but step=list is hardly ever used, and there are other steps like section_create, and so on which means authors (quite rightly) just hook into the event and do step filtering in their code.

Two ways I can see to go:

  1. As above. Plugins will break the panels and will need amending to detect the ‘criteria’ step and ignore it if they don’t require it.
  2. Change the event to something like ‘list_filter’ or ‘list_criteria’. In that case, one could argue that the step is redundant so the call could become:
$criteria .= callback_event('image_criteria', '', 0, $criteria);

although for completeness it’d probably be a good idea to keep it so the step can be targeted uniquely in case other *_criteria events are introduced in future.

Option 1 is cleaner, retaining a more consistent naming convention but with admin-side breakage. Option 2 breaks tradition but is less work for everyone to get things running under 4.5.0.

I favour option 2 because I’m lazy, but my OCD monkey says Option 1 is more ‘correct’.

Thoughts? Any other options I’ve neglected to think through? Any preference on naming convention?

Last edited by Bloke (2012-07-26 19:30:25)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#23 2012-07-26 20:06:51

Gocom
Plugin Author
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: Plugin filtering for admin list pages

In my opinion the option 1 seems close to invalid, and less cleaner than the second one. The event should be an unique in this case as with other distinct callbacks that can’t be shared. For instance this callback can’t accept any other output than SQL.

These unique compound event names are already used elsewhere, so I don’t see any problem using them. Public facing callbacks in general only use events, separating the event and step with a full stop. On admin-side, you can find underscores used as separators, e.g. article_saved and article_posted.

To me best option could be to make the event unique to the panel, while offering the step to target just specific criteria. This in the case where a panel has multiple lists or instances that need same filtering. For example for multi-edit’s assigns operations which would then need same filters as the list.

For instance:

callback_event('image_criteria', 'list');

The $criteria parameter doesn’t actually have any need, does it? It’s not like you can use it for anything or alter it.

When dealing with event names, just make sure that the name is unique and that there can not be collisions in the future. For instance, if criteria is used as the event name, then no other even can really end with term criteria. Which is why prefixing could be better option than suffixing. E.g.

callback_event('sql.criteria_{event}', '{instance}');

Or maybe the even could be shared by whole admin-side as it’s with admin_* and pluggable_ui. E.g.

callback_event('admin_criteria', 'image_list');

Last edited by Gocom (2012-07-26 20:10:13)

Offline

#24 2012-07-26 20:14:32

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,631
Website

Re: Plugin filtering for admin list pages

Gocom wrote:

callback_event(‘admin_criteria’, ‘image_list’);

Me likeee. Me adoptee.

The $criteria parameter doesn’t actually have any need, does it?

Maybe not. I was going to include it in case a plugin wanted to check the current criteria string in use so it could ‘bail out’ if perhaps another plugin had done something similar already. Long shot, but, well, umm, dunno. Is the plugin stack even cumulative or does each plugin get the “vanilla” value? Hmmm. I’ve only ever tried it with pluggable_ui() and found it not to be cumulative.

Last edited by Bloke (2012-07-26 20:17:22)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#25 2012-07-26 20:38:04

etc
Developer
Registered: 2010-11-11
Posts: 3,757
Website

Re: Plugin filtering for admin list pages

Gocom wrote:

The $criteria parameter doesn’t actually have any need, does it? It’s not like you can use it for anything or alter it.

Suppose (as per mrdale request) we want to hide some section’s articles from the list, unless this section is searched for. How a plugin would make the difference without $criteria: with gps('crit')?

Offline

#26 2012-07-26 20:52:19

Gocom
Plugin Author
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: Plugin filtering for admin list pages

Bloke wrote:

Is the plugin stack even cumulative or does each plugin get the “vanilla” value

When using callback_event(), each callback function gets the vanilla value, a (new) copy of the passed parameter. There would need to be a new callback event handler that appends callback member’s returned results to the callback parameter. If that’s even realistically possible. That’s pretty niche case after all, and locks the number and the position of parameters.

You could potentially use callback_event_ref(), but that does plain referencing. Works only if callback functions actually use the argument instead of returning statements, and members will actually be overwriting the same variable. If *_ref() is used, you would also have to create a new copy variable of the original $criteria that is then passed to the callback_event_ref(). Otherwise members will be referencing the vanilla variable and overwriting the whole SQL clause as whole.

etc wrote:

Suppose (as per mrdale request) we want to hide some section’s articles from the list, unless this section is searched for. How a plugin would make the difference without $criteria: with gps('crit')?

Well, yeah. Using the HTTP GET parameter is much more viable than the raw SQL statement. The raw SQL statement is just that; a string. It’s not very viable to use it to detect if something was searched. Unless you are going to somehow parse it or result in hackish regex.

Last edited by Gocom (2012-07-26 20:56:27)

Offline

#27 2012-07-26 20:59:47

etc
Developer
Registered: 2010-11-11
Posts: 3,757
Website

Re: Plugin filtering for admin list pages

Gocom wrote:

Using the HTTP GET parameter is much more viable than the raw SQL statement.

Agree. Then $criteria = ... is probably more natural than $criteria .= ...?

Offline

#28 2012-07-26 21:46:34

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,631
Website

Re: Plugin filtering for admin list pages

Mrdale, your pony is in the lobby

Plugin mrd_cull_section:

register_callback('mrd_cull_section', 'admin_criteria', 'list_list');

function mrd_cull_section($evt, $stp, $crit) {
	return " AND Section != 'articles'";
}

Plugin mrd_for_your_eyes_only:

register_callback('mrd_for_your_eyes_only', 'admin_criteria');

function mrd_for_your_eyes_only($evt, $stp, $crit) {
	global $txp_user;
	$user = doSlash($txp_user);

	if ($stp === 'list_list') {
		return " AND AuthorID = '$user'";
	} else if (in_array($stp, array('link_list', 'file_list', 'image_list'))) {
		return " AND author = '$user'";
	}
}

:-)

(although they probably won’t work well together, so you’d have to combine them into one plugin if you wanted both bits of functionality). You may choose to employ some extra logic to permit the Publisher to see everything.

Last edited by Bloke (2012-07-27 13:03:47)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#29 2012-07-26 21:55:57

Gocom
Plugin Author
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: Plugin filtering for admin list pages

Sweet, nice. Thanks, Stef :-)

Offline

#30 2012-07-27 11:39:16

etc
Developer
Registered: 2010-11-11
Posts: 3,757
Website

Re: Plugin filtering for admin list pages

Thank you, guys, also for having associated to your discussion someone who otherwise respects the “Experienced users only” sign. Ten-lines asv_auth_articles – wow!

But as “plugin author” I have some doubts.

  1. I would not like my where clause be concatenated with something I even do not see (debugging nightmare). Of course, I can issue something like AND 0 OR ..., but prefer avoid hacking.
  2. On the other hand, $criteria is needed in search queries, and I would not like to reconstruct it based on GET parameters (that I will forget to escape).

So, would it be possible to pass $criteria to callback_event, and let plugin authors decide what where clause should be?

Offline

Board footer

Powered by FluxBB