menu_link_maintain(), $op update critically broken.

Confirmed bug, reviewed patch, applied patch, all good...

No, I disagree - we made a choice to handle all menu links as associative arrays at all stages for consistency - hence the query should be altered to fetch associative arrays.

This is a bit ugly, but apparently is the way to do it for DBTNG:

$result = db_select('menu_links', NULL, array('fetch' => PDO::FETCH_ASSOC))

Or Crell suggest we can use:

$result->setFetchMode(PDO::FETCH_ASSOC); 

before the foreach.

Hmm, looks ok - though not sure we've used that method much - I had to look at the doxygen to see what it does - I was confused with fetchField()

Can you double-check that this is correct if there are multiple rows in the result set?

sure - guess there is no reason not to group the results.

new tests pass, looks good.

Committed to CVS HEAD. Thanks.

This is actually still broken, IMO. Since the update $op doesn't use menu_link_save(), we can't get expected results for patches like #457450: Adds hooks to track changes to {menu_links}.

Shouldn't we just be calling menu_link_save()?

OK. Here is a revised version of the patch which standardizes the 'update' op to use menu_link_save().

I think if we don't use menu_link_save() in both cases, then we have written an exception handler, not an API function.

I'm still not clear why we'd need to track the change if all that's changing is the title - are you going to update the node title based on this?

Possibly. Who knows what someone might want to do with the information. And, technically, not just nodes might use this function.

Probably premature optimization, but it would be nice to then tell menu_link_save that there are only title/options changes and that all the hierarchy info can be assumed to be correct as-is. Otherwise, we are doing a lot of work to make this change to one field. Still, I understand from a consistency standpoint why it's preferable.

I'm going to set this issue back to fixed and address this change over in #457450: Adds hooks to track changes to {menu_links}.