Allow to retrieve all filters (including disabled) in a text format [#570930]

Off-shoot from #558666: UX/security: Revamp text format/filter configuration:

The main API change, so we can revamp the user interface afterwards without having to touch the API.

Boils down to:

- Currently, filter settings are only displayed on separate pages, so we only ever need to retrieve all enabled filters.

- The new UI will remove those separate pages, which means that we need to handle enabled + disabled filters on the same page.

- Since that just means that we also want to get any additional, disabled filters, we simply add an argument to filter_list_format() that allows to retrieve all filters in a format, regardless of whether they are enabled.

I'm confident that #558666: UX/security: Revamp text format/filter configuration will hit core in the next few weeks, so this really is the API change only. The added foreach in the form submit handler will vanish when the other patch will be committed.

Comment	File	Size	Author
#4	drupal.filter-list-format.4.patch	9.13 KB	sun
#4
#2	drupal.filter-list-format.2.patch	6.76 KB	sun
#2
	drupal.filter-retrieve-all.patch	3.16 KB	sun

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

David_Rothstein CreditAttribution: David_Rothstein commented 7 September 2009 at 23:06

Status:

Needs review

» Needs work

Seems to make sense. I assume that as a minor API improvement, this could happen during the "code slush" period as well (i.e., does not need to get committed today)?

I think there is a bug, though. If you call filter_list_format() with $all = TRUE and then later call it with $all = FALSE, the second time will return the wrong information from the static cache. Seems like there needs to be a separate cache for the different values of $all.

Also, for the $all parameter, I'd suggest something more like this:

 * @param $include_disabled_filters
 *   (optional) A boolean which can be set to TRUE to retrieve all filters
 *   associated with the given format, including those which are disabled.
 *   Defaults to FALSE, meaning that only enabled filters will be retrieved.

Comment #2

sun

German

Karlsruhe

CreditAttribution: sun commented 10 September 2009 at 15:35

Status:

Needs work

» Needs review

File	Size
drupal.filter-list-format.2.patch	6.76 KB

Thanks!

I've merged some further clean-ups from #558666: UX/security: Revamp text format/filter configuration as well as #11218: Allow default text formats per role, and integrate text format permissions into this patch. Those two need to be re-rolled anyway.

While working on this patch, I realized that $filter is sometimes an array and sometimes an object. That should be consistent, and we should create separate issue for that (tagged with FilterSystemRevamp).

Comment #3

10 September 2009 at 15:50

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #4

sun

German

Karlsruhe

CreditAttribution: sun commented 10 September 2009 at 16:02

Status:

Needs work

» Needs review

File	Size
drupal.filter-list-format.4.patch	9.13 KB

Fixing the tests.

Comment #5

sun

German

Karlsruhe

CreditAttribution: sun commented 10 September 2009 at 16:09

Issue tags:

+API clean-up

Introducing a new tag for feature freeze: API clean-up.

Comment #6

Dries CreditAttribution: Dries commented 11 September 2009 at 07:20

Status:

Needs review

» Reviewed & tested by the community

This is a necessary API clean-up so I'm going to let this one go in.

+++ modules/filter/filter.module	10 Sep 2009 15:31:25 -0000
@@ -472,47 +474,52 @@ function filter_format_allowcache($forma
+function filter_list_format($format, $include_disabled = FALSE) {

How about $active_only instead of $include_disabled? This is a really minor suggestion. I'm also happy with $include_disabled. My original thought was, why not return all filters all the time with a status field, and have the caller sort it out, but this is probably a bit faster and makes the calling code slightly more elegant.

Happy to commit this 'as is' later today.

(PS: it looks like Dreditor picked up the wrong function in the snippet above.)