The "Manual updates required" in admin/modules/update is generated in the wrong format, and as a consequence has a spurious column:

Files: 
CommentFileSizeAuthor
#16 1118404-by-Damien-Tournoud-keichee-bfroehle-Fixed-Ma.patch1.47 KBbfroehle
PASSED: [[SimpleTest]]: [MySQL] 32,783 pass(es).
[ View ]
#5 1118404-update-manual-table-broken.patch644 byteskeichee
PASSED: [[SimpleTest]]: [MySQL] 32,208 pass(es).
[ View ]
#1 1118404-update-manual-table-broken.patch1.13 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 29,390 pass(es).
[ View ]
manual-updates-required.png29.43 KBDamien Tournoud

Comments

Status:Active» Needs review
StatusFileSize
new1.13 KB
PASSED: [[SimpleTest]]: [MySQL] 29,390 pass(es).
[ View ]

It's not really pretty, but there is nothing better we can actually do :(

Status:Needs review» Closed (duplicate)

I'm marking this as a duplicate of #1024990: "Manual updates required" table has an extra column, however you solve the problem in a different way. I'll like that other issue to this one.

Status:Closed (duplicate)» Reviewed & tested by the community
Issue tags:+needs backport to D7

The patch in #1 is likely the simplest fix to this problem. I've tested it out and it works wonders. :) I marked the other issue, referred to in #2, as a duplicate of this one.

Status:Reviewed & tested by the community» Needs review

Is the following summary correct?

We have a function that builds up an array that is either passed into theme('tableselect') or theme('table').

The problem is that theme('tableselect') and theme('table') take a different array as input.

This hack turns a theme('tableselect') array into theme('table') array.

If it is correct, I think we can better document it in the code. Setting back to 'needs review' as I'd like some extra input.

Assigned:Unassigned» keichee
Priority:Normal» Critical
Status:Needs review» Patch (to be ported)
StatusFileSize
new644 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,208 pass(es).
[ View ]

this is a much simpler way that works..

Priority:Critical» Normal
Status:Patch (to be ported)» Needs review

Patch looks good, but this isn't critical, and the patch doesn't look like it needs porting to me, ought to apply straight.

Tested this manually, fixes the problem fine. I think we could still use a comment here though - the code is much simpler but we should explain why we're unsetting attributes.

thats per entry, so core entry attributes have no attributes set, others will have.

thats a simple issue that no one will reuse and free some space.

Dries' hypothesis in #4 is correct.

#5 really looks like a hack.

Status:Needs review» Needs work

@Damien, without inline docs, both #1 and #5 look like hacks, except #5 is a lot more concise. So if #1 is the correct fix here and #5 is a nasty workaround, the docs should make that obvious. Marking CNW since neither are adding those.

Just needs a comment like

+    // The elements of the projects array are each generally themed as a
+    // tableselect giving checkboxes to chose which updates to install.
+    // To omit these checkboxes for projects where 'Manual updates required',
+    // we instead theme this section as a table. Unfortunately the variables
+    // for theme_tableselect and theme_table are not compatible, so we must
+    // first massage the data into an appropriate format.

look, i just followed the existing code to handle core

existing code is

      unset($entry['#weight']);

but after i traced, weight is not even existing, i think weight is a typo and it must be an attributes.

try to print_r the $entry variable outside that if condition code that handle manual updates and see what i mean.

print_r($entry);

my code is legit as per complying to the existing code how to handle manual updates.

The patch in #1 with a comment similar to #12 should suffice.

Status:Needs work» Needs review
StatusFileSize
new1.47 KB
PASSED: [[SimpleTest]]: [MySQL] 32,783 pass(es).
[ View ]

I've updated the comment patch in #1 to address Dries' concern in #4.

#5 is a hack in the sense that it unnecessarily removes the attributes (i.e., 'class' => 'update-recommended') which may be used by the site's CSS.

Status:Needs review» Reviewed & tested by the community

Marking this as RTBC since it is the same code as in #1 (which was marked RTBC in #3) and hopefully addresses Dries concerns with the comments.

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

The new comment covers this well I think.

Committed to 8.x, moving back to 7.x for webchick to look at.

Status:Reviewed & tested by the community» Fixed

w00t! Great to see this fixed!

Committed and pushed to 7.x.

Status:Fixed» Closed (fixed)
Issue tags:-needs backport to D7

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