The attached patch;-

  • Adds two classes to the markup
    'number':
    So that numbers can be right aligned
    'warning':
    So non-zero values of certain keys, can be highlighted
  • Provides the css styles to render appropriately

I was prompted to do this after finding incorrect description http://drupal.org/node/107375.

Patch should apply with -p0, possibly against HEAD, but was made against 6.0 RC2.

CommentFileSizeAuthor
#31 status.patch726 bytescatch
#25 issue-212233.patch4.18 KBlilou
#18 sql_status_7.x.patch4.36 KBlilou
#16 numeric_column.patch955 byteszeta ζ
#17 sql_status_.patch4.32 KBzeta ζ
#15 sql_status_.patch4.25 KBzeta ζ
#13 SQL.patch3.32 KBzeta ζ
#12 sql_status.patch3.21 KBzeta ζ
#8 pretty_status.patch1.59 KBzeta ζ
#7 pretty_status.patch1.55 KBzeta ζ
#5 sytem.admin_.inc_.patch833 byteszeta ζ
#5 SQL_report.png32.17 KBzeta ζ
#3 SQL | localhost_1201191514420.png79.62 KBzeta ζ
#1 pretty_status.patch1.49 KBzeta ζ
pretty_status.patch1.47 KBzeta ζ
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
pretty_status.patch1.49 KB

Added another key.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

How would the improved interface look compared to the old?

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
SQL | localhost_1201191514420.png79.62 KB

The Value column: right aligned.
Non-zero values of ‘should be zero’ highlighted.

Both done with classes, so that themes can override.
See attached for default.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

I know the goal here is to get attention to the bad value, but this looks clunky. The yellow box is quite big around the number (I know it is the table cell, but it looks odd).

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
Status: Needs work » Needs review
FileSize
SQL_report.png32.17 KB
sytem.admin_.inc_.patch833 bytes

Yes, I agree, but I wanted to avoid extra markup.

What about this – I’ve borrowed an icon from watchdog.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

Looks much better IMHO, but you posted the wrong patch.

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
Status: Needs work » Needs review
FileSize
pretty_status.patch1.55 KB

Sorry…

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
pretty_status.patch1.59 KB

This is better, as it applies class .warning to whole row, the way watchdog does it.

Haven’t used the (not so) yellow background that watchdog applies to the row, so looks the same as in #5 *.png .

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

tr .number seems buggy, since it would apply to the row, while your intention is to apply it to a cell, right?

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented

tr .number is the class of a td within a row. tr td.number would be more explicit, but unnecessary, and I think I remember reading that it is less efficient.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review

Indeed, sorry for missing the space there :)

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
Version: 6.0-rc2 » 6.x-dev
FileSize
sql_status.patch3.21 KB

I’ve re-worked this and it will now apply against HEAD (not sure if that is 6.x-dev or 7.x-dev). Will also apply to D6.2 with offsets (-49) if you use patch -p1 -F3 .

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
Version: 6.x-dev » 7.x-dev
FileSize
SQL.patch3.32 KB

Re-rolled against HEAD. I can roll against 6.3 or -dev if needed. Fairly easy to port to 5 too.

I kmow it’s only a task, but it would be great to get this in as http://drupal.org/node/107375 is already there: picked that one up after a year languishing as ‘needs work’.

Dries’s picture

Dries CreditAttribution: Dries commented

This patch seems to generate NOTICES when I try it.

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
sql_status_.patch4.25 KB

Sorry Dries, mended now :-) using isset() (and E_ALL). Obviously I’m still getting used to PHP’s foibles.

Note:
I’ve just posted http://drupal.org/node/284721 which also relates to this code
zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
numeric_column.patch955 bytes

This patch could be applied after the above (Edit: or the below). It moves the ’right alignment’ to system.css, from admin.css, so that tables in general can have numeric columns styled by adding class="numeric" to each <td> or <th>

zeta ζ’s picture

zeta ζ CreditAttribution: zeta ζ commented
FileSize
sql_status_.patch4.32 KB

Re-roll after commit of http://drupal.org/node/284721

lilou’s picture

lilou CreditAttribution: lilou commented
FileSize
sql_status_7.x.patch4.36 KB

Re-roll;

lilou’s picture

lilou CreditAttribution: lilou commented
Status: Needs review » Needs work

Patch #18 no longer applies against CVS/HEAD.

Dries’s picture

Dries CreditAttribution: Dries commented

Can we get a fresh screenshot too? :)

lilou’s picture

lilou CreditAttribution: lilou commented
Title: Improve UI of SQL status report » Kill SQL status report ?

Perhaps it's time to kill system_sql() now drupal support SQLite and Postgresql ?

/**
 * Menu callback: return information about the database.
 */
function system_sql() {

  $result = db_query("SHOW STATUS");
  while ($entry = db_fetch_object($result)) {
    // 'SHOW STATUS' returns fields named 'Variable_name' and 'Value',
    // case is important.
    $data[$entry->Variable_name] = $entry->Value;
  }

return Notice: Undefined property: stdClass::$Value in system_sql() (line 1851 of D:\Serveur\www\drupal\7.x\modules\system\system.admin.inc).

lilou’s picture

lilou CreditAttribution: lilou commented
Title: Kill SQL status report ? » admin/reports/status/sql : kill SQL status report ?
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

We should remove it. If we end up adding back in a database report at a later date all well and good.

lilou’s picture

lilou CreditAttribution: lilou commented
Assigned: zeta ζ » lilou

OK, i take it.

lilou’s picture

lilou CreditAttribution: lilou commented
Title: admin/reports/status/sql : kill SQL status report ? » admin/reports/status/sql : remove SQL status report
FileSize
issue-212233.patch4.18 KB
lilou’s picture

lilou CreditAttribution: lilou commented
Status: Needs work » Needs review

see also #423926: Restore database information screen

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community

Here's how it looks at the moment.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Ok, let's nuke it -- it could be a good feature for devel module. Committed. Thanks!

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Fixed » Needs work

Please also nuke the db_status_report() call in system_requirements().

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review
FileSize
status.patch726 bytes
lilou’s picture

lilou CreditAttribution: lilou commented
Status: Needs review » Reviewed & tested by the community

Looks good.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.