update.php displays misleading messages [#1013808]

Steps to reproduce:

1) Install a module or core and enable it (eg, drupal-7.0-beta1).
2) Update to a new version of module or core (eg, drupal-7.0-rc4).
3) Run update.php successfully.
4) Receive screenshot (before.jpeg).

With the attached patch, you aren't told that modules returned messages when they didn't. See after.jpeg. Any failed updates (or returned messages of any type) would still display properly, along with the module name.

The "Returned Messages" box still appears even if there were a no messages - but to fix that would require a bit of reorganization in the function as a whole. Thus, the one line patch - it's not perfect, but it removes a major UX wtf moment with very little possibility of breaking anything.

Comment	File	Size	Author
#10	after2.jpeg	18.07 KB	threewestwinds
#8	1013808-update_output-2.patch	2.16 KB	redndahead
#8
#8	current_one_with_message_one_without.png	9.74 KB	redndahead
#8	current_two_messages.png	11.71 KB	redndahead
#8	patch8_one_with_message_one_without.png	8.34 KB	redndahead
#8	patch8_two_messages.png	11.83 KB	redndahead
#6	after.jpeg	37.31 KB	threewestwinds
#6	after1.jpeg	17.74 KB	threewestwinds
#4	1013808-update_output-1.patch	2.16 KB	redndahead
#4
#3	update-messages.patch	1.84 KB	threewestwinds
#3
#1	DatabaseUpdateResultPage.png	34.41 KB	Tor Arne Thune
	update-messages.patch	1007 bytes	threewestwinds

	after.jpeg	22.39 KB	threewestwinds
	before.jpeg	33.39 KB	threewestwinds

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Tor Arne Thune CreditAttribution: Tor Arne Thune commented 3 January 2011 at 22:31

File	Size
DatabaseUpdateResultPage.png	34.41 KB

I can confirm that the patch works as described. Updated token 7.x-1.0-alpha2 to token 7.x-1.0-alpha3 and got the screen attached after running the database updates.

Comment #2

Bojhan CreditAttribution: Bojhan commented 3 January 2011 at 22:52

An empty box, lets just not display it then.

Comment #3

threewestwinds CreditAttribution: threewestwinds commented 3 January 2011 at 23:21

File	Size
update-messages.patch	1.84 KB

Empty box hidden.

Comment #4

redndahead CreditAttribution: redndahead commented 4 January 2011 at 02:58

File	Size
1013808-update_output-1.patch	2.16 KB

Ok reviewed this and the latest patch seems like it would print like this

These modules have messages

Views
Query 7001
blah

Views
Query 7002
blah blah

Webform
Query 7001
blah

This patch changes it to.

These modules have messages

Views
Query 7001
blah

Query 7002
blah blah

Webform

Query 7001
blah

This does need a look over to make sure output is correct.

Comment #5

4 January 2011 at 03:41

Status:

Needs review

» Needs work

The last submitted patch, 1013808-update_output-1.patch, failed testing.

Comment #6

threewestwinds CreditAttribution: threewestwinds commented 4 January 2011 at 04:51

Status:

Needs work

» Reviewed & tested by the community

File	Size
after1.jpeg	17.74 KB
after.jpeg	37.31 KB

EDIT: Correction: Those images are with #3 applied. Please read with this post with the fact in mind that I thought I was testing #4, but it was actually my own patch that I had applied (doh).

Here's a screenshot of a bunch of updates with #4 applied. There were many updates that returned nothing, and those modules (properly) did not display anything. Notice however that "taxonomy module" shows up twice.

The second screenshot again has a bunch of successful updates, this time without core hacked to return a bunch of junk. :) Notice the complete lack of misleading or empty text boxes.

So, it doesn't fix every bug in this screen - but it's still much better than what went before, and it does fix the opening issue that both @merlinofchaos, @webchick and I had noticed. Thus, RTBC, with the intention of a followup issue for 7.1 to address the (corner case) doubling of certain module names - it only comes up when a module has two updates run at once, and even then only id they both succeed and return messages.

Comment #7

threewestwinds CreditAttribution: threewestwinds commented 4 January 2011 at 03:57

Cross posted with bot.

Comment #8

redndahead CreditAttribution: redndahead commented 4 January 2011 at 05:09

Status:

Reviewed & tested by the community

» Needs review

File	Size
patch8_two_messages.png	11.83 KB
patch8_one_with_message_one_without.png	8.34 KB
current_two_messages.png	11.71 KB
current_one_with_message_one_without.png	9.74 KB
1013808-update_output-2.patch	2.16 KB

This patch fixes a bad variable name and switching single quotes to double quotes for new line. Also here are some screenshots of it working. I'll be happy to post any additional screenshots you need.