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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Status: Active » Needs review
FileSize
1.13 KB

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

bfroehle’s picture

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.

bfroehle’s picture

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.

Dries’s picture

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.

keichee’s picture

Assigned: Unassigned » keichee
Priority: Normal » Critical
Status: Needs review » Patch (to be ported)
FileSize
644 bytes

this is a much simpler way that works..

catch’s picture

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.

catch’s picture

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.

keichee’s picture

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.

bfroehle’s picture

Dries' hypothesis in #4 is correct.

Damien Tournoud’s picture

#5 really looks like a hack.

catch’s picture

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.

bfroehle’s picture

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.
keichee’s picture

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.

bfroehle’s picture

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

bfroehle’s picture

bfroehle’s picture

Status: Needs work » Needs review
FileSize
1.47 KB

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.

bfroehle’s picture

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.

catch’s picture

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.

webchick’s picture

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.