the attached patch makes dblog module to support two new hooks log_filters and log_filters_validate
this will allow other modules to provide their own filters to improve "Recent log" report filtering

as an example dblog_time_filters (also attached) is a module taking advantage of this which provides filter capability by "recent" and "aged" being both configurable (an array of integers interpreted as minutes is formated as a more readable text)
to illustrate this, the image attached shows dblog_time_filters module taking advange of log_filters and log_filters_validate hooks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

arhak’s picture

the demo module attached above (dblog_time_filters) addresses time filtering
not same but similar to what requests #251814: Add time based filter on watchdog overview page

this patch will facilitate implementations of any filter for log entries

arhak’s picture

Status: Needs review » Needs work

The last submitted patch failed testing.

arhak’s picture

rerolled with webchick's changes

arhak’s picture

Status: Needs work » Needs review
NancyDru’s picture

subscribing

arhak’s picture

Issue tags: +Usability
arhak’s picture

FileSize
10.21 KB

I have already implemented a few log filters by means of custom modules taking advantage of this patch to dblog

having dblog to invoke hooks allows to provide filters (which might be enabled/disabled at will)
- filter by username (with autocompletion)
- filter by hostname
- filter by recent (configurable recent times)
- filter by aged (ditto)
- filber by date & time around +/- X time (not yet implemented)

the attached image shows two new filters taking advantage of hooks provided by this patch

Note: the UI changes are NOT part of this patch, this patch ONLY provides hooks for dblog module to look for the enabled filters

arhak’s picture

for those reviewing usability
screenshots are available showing what modules would be able to provide through these hooks on dblog
taking advantage of log_filter hook

Bojhan’s picture

wow, what the hell. I would love to review usability, but I think its best if you first take a shot at it yourself. Obviously having 4 large select lists, totally collapsed with actions floating beside them is not the best way to go. Consider the use case, which ones do you use the most? And how can you align them all so that they make sense, perhaps you need to collapse them.

Bojhan’s picture

Also, we are introducing new functionality here - that I am questioning, perhaps someone else could review its importance.

Dries’s picture

I'm not in need of 'Aged' and 'Recent' filters, but boy, I'd love 'Username', 'Hostname' and 'Description' filters in core.

Dries’s picture

Cross-posted with Bojhan.

NancyDru’s picture

Dries: +1 on username. This is one of the things I suggested in the Util issue that sparked this. And I think if you had "Aged" you might actually use them once in a while, especially if you used Util's log_clear add-on, which allows selective clearing of log entries.

Hierarchical select somehow adds a resizing "grippie" to its select lists, perhaps that would be a good idea here to minimize the "clutter".

NancyDru’s picture

I have tried this and everything looks good, except I'd like a tiny bit of space between the selection boxes, but that should not hold this up. IMO, RTBC.

#dblog-filter-form #edit-filters .form-item {
  margin-right: 2px;
}

BTW, Dries, here's your user filter:

/**
 * Implement hook_log_filters().
 * 
 * List dblog administration filters that can be applied.
 */
function user_log_filters() {
  $filters = $users = array();

  $result = db_query('SELECT DISTINCT(u.name), u.uid FROM {watchdog} w INNER JOIN {users} u ON u.uid = w.uid ORDER BY u.name');
  foreach ($result as $object) {
    $users[$object->uid] = $object->name;
  }

  if (!empty($users)) {
    $filters['users'] = array(
      'title' => t('user'),
      'where' => "w.uid = ?",
      'conjunction' => TRUE,
      'options' => $users,
    );
  }

  return $filters;
}

If you'd prefer it as a patch to the user.module, I can handle that.

BTW, also see #583632: Clear log description needs clarification

arhak’s picture

@#10 please consider that this patch just "opens" the possibility to other modules to do this,
the actual filters are (by now) provided by modules out of dblog
therefore, how things gets laying out, or how it is styled was not addressed

if would be a "need" to have other filters, the first thing to do is to allow them with a hook, right?

do you think some of the filters in the screenshot should be in dblog from the beginning?
it can be done, but I recommend to keep "type" and "severity" being mandatory filters
and being available to get ride of any/every remaining filter (except for type & severity)

EDIT: typos

NancyDru’s picture

@arhak: I'm thinking that perhaps the dblog module should come with an "extra filters" module that provides the add-ons, and maybe with a UI to select which ones to enable. But that may be asking too much for 7.x at this late date; we can always include that in the Util module (or a new contrib) until 8.x.

arhak’s picture

@#17 totally agree BUT

wait, please, that is exactly what I would like, yes, the filters them self got too late and would be better provided by dblog_filters as already did for D6

BUT instead of requiring a whole module to replace dblog's functionality

WHAT I'm requesting and the ONLY thing this patch is asking for are the hooks to be able to do that

in this comment 578488#comment-2064964 you can see that there are several modules involved
the ONLY one I want to get into core would be dblog_ext (the one providing the hook by means of duplicating dblog functions just to put some hooks in it)

arhak’s picture

the UI changes and the D6 modules might help deciding whether this would be a desirable issue to get into D7
BUT the actual proposed change is just: "provide hooks for filters support"

other modules will take advantage of those hooks to be the ones actually providing the filters

arhak’s picture

Title: dblog support filters through a hook » provide hook for dblog_filters
arhak’s picture

FileSize
3.41 KB
31.05 KB

why this patch to dblog?

- check the attached image for illustration

- this patch provides 2 hooks: log_filters and log_filters_validate
(note that both hooks are prefixed with "log" instead of "dblog" since their are applicable to logs whether or not they are for "Recent log" report)

- with this two hooks implementing new optional filters to "Recent log" report would be very easy

- as an illustrative demostration there are two working modules attached (for D7)
-- one is dblog_filters which facilitates other modules to provide filters for "Recent log" report through regular log_filter hooks, since hooking into log_filters won't make autocompletion widgets available at core for instance, so this module provides the way
-- the other one is dblog_user_filter (with autocompletion)

how simple is dblog_user_filter?
as simple as the following code (is the whole module)

/**
 * Implementation of hook_log_filters_settings().
 */
function dblog_user_filter_log_filters_settings() {
  $settings['username'] = array(
    'title' => 'Username',
    'description'    => 'Filters by username with autocompletion.',
    //'page arguments' => array('dblog_user_filter_settings'),
    //'file'           => 'dblog_filters.user.admin.inc',
  );
  return $settings;
}

/**
 * Implementation of hook_log_filters_widget().
 */
function dblog_user_filter_log_filters_widget() {
  $filters['username'] = array(
    'title' => t('Username'),
    'where' => 'u.name = ?',
    'conjunction' => FALSE,
  );
  $filters['username']['widget'] = array(
    '#type' => 'textfield',
    '#autocomplete_path' => 'user/autocomplete',
    '#maxlength' => USERNAME_MAX_LENGTH,
    '#size' => 27, // fit the same with than <select>
  );
  return $filters;
}

PS: neither of the attached modules will work properly without this patch, since dblog_filters relies on it

arhak’s picture

Issue tags: +Needs usability review

for those reviewing usability
check why_dblog_patch.png for a visual explanation

this patch does NOT provides UI changes, ONLY provides new hooks
which in turn will allow other modules to enhance log filtering

sinasalek’s picture

+1 subscribed

arhak’s picture

Status: Needs review » Needs work

this one needs to be re-rolled (I'll upload it soon)

two notes:
- current patch proposes a CSS selector change for dblog.css which has to be done for dblog-rtl.css as well
- currently dblog.admin.inc contains a bug at line 279, since variable $session does not exist any more, it should be $_SESSION['dblog_overview_filter'] (current patch used TRUE until now)

function dblog_filter_form($form) {
  $filters = dblog_filters();

  $form['filters'] = array(
    '#type' => 'fieldset',
    '#title' => t('Filter log messages'),
    '#theme' => 'dblog_filters',
    '#collapsible' => TRUE,
    '#collapsed' => empty($session), // <-- this variable was suppressed
  );
arhak’s picture

Status: Needs work » Needs review
FileSize
6.31 KB

- new re-rolled patch ONLY differ in having dblog-rtl.css modified the same way dblog.css got modified
- dblog_user_filter module (uploaded @#21) launches some warnings (Invalid argument supplied for foreach() in theme_admin_page()) because it requires to be updated, BUT for testing purposes it works (just remember to visit admin/config/development/logging/filters to enable the filter after having dblog_user_filter module enabled)
- dblog.css & dblog-rtl.css are working (seeing through Firebug) but module's stylesheets are being overrided by theme's reset.css (with or without this patch)
- the above comment (@#24) about unexistent $session variable that should be $_SESSION['dblog_overview_filter'] is right for the existing bug in dblog module, but this patch initializes 'collapsed' => TRUE, and latter it expands filters' fieldset if at least one filter has a saved value (code is now commented)

    if (!empty($_SESSION['dblog_overview_filter'][$key])) {
      $form['filters']['status'][$key]['#default_value'] = $_SESSION['dblog_overview_filter'][$key];
      // expand filters fieldset if at least one filter has a saved value
      $form['filters']['#collapsed'] = FALSE;
    }
arhak’s picture

BTW: the above comment about module's stylesheet being overrided by theme's reset.css is an issue of theme Seven, those CSS can be successfully tested with Garland

EDIT: leaving a reference to the Seven issue regarding this CSS problem which is more notably when testing this patch but is present with or without this patch #674208: style.css dismisses module's css

Status: Needs review » Needs work

The last submitted patch failed testing.

arhak’s picture

Test bed refused windows line endings.

Ensure the patch only contains unix-style line endings.

arhak’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, , failed testing.

Dave Reid’s picture

Status: Needs work » Needs review

Re-test of from comment #2393924 was requested by @user.

Status: Needs review » Needs work
Issue tags: +Needs usability review, +Usability

The last submitted patch, , failed testing.

arhak’s picture

this patch has nothing to do with FileFieldValidateTestCase which was the cause of the failing 2 tests and 14 exceptions (http://qa.drupal.org/pifr/test/23044)

lets wait until that get fixed to hit the bot again

arhak’s picture

Status: Needs work » Needs review

I checked the logs around the failing test (http://qa.drupal.org/pifr/log/recent) and found other 2 tests (http://qa.drupal.org/pifr/test/23068 & http://qa.drupal.org/pifr/test/23268) which also failed due to upload problems plus exceptions (while the patches are unrelated with FileField).

But it doesn't seems to be usual, since other tests got through.
So... hitting the bot again...

Re-test of from comment #2393924 was requested by @user.

arhak’s picture

int opened an issue for the bot failure #667264: [TESTING BROKEN] FileFieldValidateTestCase fails randomly

EDIT: the test of this issue already passed, leaving the above reference in case it comes up again

NancyDru’s picture

Does that mean this should be RTBC?

arhak’s picture

@NancyDru
it might..
test bot is green, I tested it, you tested it, anyone else?
did Dries and/or Bojhan test it?

arhak’s picture

Status: Needs review » Needs work

comment #25 and more specifically #26 mention a CSS bug with Seven but not with Garland
the actual problem is within this patch:
dblog.css & dblog-rtl.css got their first CSS selector modified and I missed the selector prefix #dblog-filter-form which is causing Seven's style.css to achieve a narrower selector by means of div.form-item (which shouldn't happen with this patch)

Nevertheless, that's a minor flaw I will fix soon, the patch still needs review (which can be happily achieved with Garland)

arhak’s picture

Status: Needs work » Needs review

@#40 NOT this patch's fault

-.form-item-type, .form-item-severity {
+.form-type-select {

the existing selectors are not working with Seven, which is getting a narrower selector with div.form-item, so currently dblog module has this error when using Seven and I shouldn't mix that fix within this patch

what would need consideration is using .form-type-select or .form-item
I used the most specific following what was already there for -type and -severity
but probably #dblog-filter-form .form-item would fit better

Note: for this to be appreciated in Seven (with or without this patch) the first selector in dblog.css & dblog-rtl.css has to be preceded by #dblog-filter-form (with a space before .form-*)

EDIT: typo fix

arhak’s picture

follow up:
the issues discovered while testing this patch (mentioned at #24, #25, #40, #41) got their separated issues (which if get fixed will require a re-roll of this one)
#674352: variable $session does not exist any more
#674354: CSS selectors get overridden by narrower selector at style.css when using Seven

Everett Zufelt’s picture

Correcting tags

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs usability review, +Usability, +Needs accessibility review

The last submitted patch, 2009-12-15_dblog_log_filters_hook04unix.patch, failed testing.

arhak’s picture

Status: Needs work » Needs review
FileSize
6.11 KB

rerolled same patch as #28
BUT with no CSS support for potential new filters (that would be another issue)

arhak’s picture

@#12 description wouldn't be a possible filter as long as watchdog saves pairs of 'message', $args for on-display-time translation (can't evaluate every translatable string on the fly for the current language to make a selection of them)
there are also the already translated logs which got pairs of t('message'), NULL that can be selected in the table, but they might be in several languages depending of the current language at the time they got into watchdog

I guarantee a new project dblog_filters taking care of username/hostname once this patch gets in (otherwise it would be the madness of reproducing dblog.admin.inc in a dblog_ext module, as done in util-6.x-3.x)
so it would be available for future core inclusion consideration (if it proves its worth)

arhak’s picture

I was reviewing this patch one more time, and found a couple of bothering unclearness
- now using 'multiple' (instead of odd 'conjunction')
- less modified code, aiming to be loyal to legacy code (works exactly the same though)

arhak’s picture

attached modules for testing this patch
please, use one at the time to avoid overloaded filters form
plus the username autocompletion is lacking some CSS for better fitness

also attached a CSS patch to make all select list properly float left
(which on the side solves #674354)
Note that the CSS patch was renamed as being for D6 to avoid test bot
since it is ONLY intended to be used for testing the patch of this issue with a more confortable CSS layout

Log time filters (dblog_time_filters):
- relies on hook_log_filters
- exposes recent/aged filters
- dependencies[] = dblog
- not relying on dblog_filters' API

Log filters (dblog_filters)
- relies on hook_log_filters
- provides an API easing custom filter implementations
- dependencies[] = dblog

Log hostname filter (dblog_host_filter)
- exposes hostname/IP filter
- dependencies[] = dblog_filters

Log user filter (dblog_user_filter)
- exposes username filter
- dependencies[] = dblog_filters

arhak’s picture

screenshots (modules with CSS patch at #49 for testing)
the actual patch of this issue is at #48

arhak’s picture

Assigned: Unassigned » arhak
retester2010’s picture

Status: Needs review » Needs work
+++ modules/dblog/dblog.admin.inc	Wed Mar 10 03:41:07 2010 -0500
@@ -193,11 +193,24 @@ function dblog_build_filter_query() {
+    }
+    ¶
+    // Non multiple filters will have a single value list

trailing white space

Powered by Dreditor.

mgifford’s picture

@retester2010 is this against patch #48

Think it's ready for RTBC after that space is removed?? I can re-roll this if it helps move things ahead.

mgifford’s picture

Status: Needs work » Needs review
Issue tags: -Needs usability review, -Usability, -Needs accessibility review

Status: Needs review » Needs work
Issue tags: +Needs usability review, +Usability, +Needs accessibility review

The last submitted patch, 2010-03-10_582622_dblog_log_filters_hook06unix.patch, failed testing.

Dave Reid’s picture

Version: 7.x-dev » 8.x-dev

Bumping to D8. It's too late for an API change.

arhak’s picture

Assigned: arhak » Unassigned
klonos’s picture

subscribing...

Bojhan’s picture

Issue tags: -Needs usability review

Whoa, this is quite messy - what are we doing here?

Nahbyr’s picture

Just out of curiosity, is this still being developed or already finished?
This would be a handy feature.
If it's already finished then thanks to whoever made this! If not then I'll probably have a go at it myself.

EDIT: Just to clarify, I meant the Drupal 8 bit.

arhak’s picture

@Nahbyr#60
AFAIK this issue is unattended
with all due respect, Bojhan (@#10 & @#59) keeps wondering what this is about, despite my attempts to make it the most self-explanatory as possible

this is a request to introduce a harmless hook, in order to allow contrib to extend dblog's filters functionality
http://drupal.org/files/issues/why_dblog_patch.png

mgifford’s picture

Issue summary: View changes

@arhak Are you willing to subit this as a patch to D8?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

cilefen’s picture

Version: 8.1.x-dev » 8.2.x-dev
Category: Support request » Feature request

This is a miscategorized feature request.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dagmar’s picture

Status: Needs work » Closed (won't fix)

Now that #2015149: Replace dblog recent log entries with a view finally landed this issue can be closed. In my opinion adding hooks to alter the dblog_filters never was a viable alternative because you have to alter a lot of other parts of the dblog module to make this work.