Allow switch user block to show only autocomplete field [#599196]

On the "Switch user" block, I want the option to hide the list of users, and only show the autocomplete form. However, if "number of users to display in the list" is configured at 0, then the block doesn't appear at all.

I'm submitting a patch which displays the block when "number of users" is 0, so that the autocomplete form remains. If someone wants the block to disappear, disabling it is better and faster, since it avoids the unnecessary processing and database queries.

Additionally, I couldn't find any obvious reason to for the logic in devel_switch_user_list() to have its own function, so I merged it with the devel_block_switch_user() function. If that is a problem, I can roll another patch to maintain the two separate functions.

Comment	File	Size	Author
#10	devel-7.patch	719 bytes	joelstein
#10
#9	devel-6.patch	719 bytes	joelstein
#9
#7	devel.module-d6.patch	375 bytes	joelstein
#7
#6	devel.module-d6.patch	206 bytes	joelstein
#6
#6	devel.module-d7.patch	403 bytes	joelstein
#6
#4	devel.module-d6.patch	739 bytes	joelstein
#4
#4	devel.module-d7.patch	867 bytes	joelstein
#4
#1	devel.module.patch	5.01 KB	joelstein
#1
	devel.module.patch	4.64 KB	joelstein

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

joelstein CreditAttribution: joelstein commented 8 October 2009 at 16:04

Version:	6.x-1.9	» 6.x-1.18
Status:	Active	» Needs review

File	Size
devel.module.patch	5.01 KB

Sorry, I realized that I tagged this with the wrong version. Also, I added a field description on the block configuration form to explain this new behavior.

Comment #2

salvis

he/his

CreditAttribution: salvis commented 24 October 2009 at 16:26

Status:

Needs review

» Needs work

The separate devel_switch_user_list() function might be useful to a module like admin_menu. I don't see eliminating this function as an improvement.

Your patch should be against the -dev version, otherwise we can't apply it.

The patch is surprisingly big. I agree with you that the current behavior is not useful, but I'd expect your change to be only a few lines...

Remember that our focus is at least as much on D7 as on D6. We won't commit a patch to D6 and not to D7. If your patch were really short and sweet, you might find someone willing to port it to D7, but this is not likely for a big one like yours. Your best bet for getting this in is to provide two slim patches against both -dev versions.

Comment #3

joelstein CreditAttribution: joelstein commented 28 October 2009 at 14:35

Well, it looks big because I merged the two functions. But it's really quite simple. I'll resubmit a slimmer patch against dev. Thanks for the feedback.

Comment #4

joelstein CreditAttribution: joelstein commented 28 October 2009 at 15:15

Version:	6.x-1.18	» 6.x-1.x-dev
Status:	Needs work	» Needs review

File	Size
devel.module-d7.patch	867 bytes

devel.module-d6.patch	739 bytes

Here's two slimmed-down patches.

Comment #5

salvis

he/his

CreditAttribution: salvis commented 28 October 2009 at 16:44

It still seems overly complex.

  if (!empty($links) || user_access('switch users')) {

should do the same thing, no?

I have some doubts whether theme('links', array()); is safe though... Anyone?

Comment #6

joelstein CreditAttribution: joelstein commented 11 November 2009 at 23:05

File	Size
devel.module-d7.patch	403 bytes

devel.module-d6.patch	206 bytes

Okay, here are the two patches, both just one line long. Sending theme_links() a blank array is safe (have a look at http://api.drupal.org/api/function/theme_links -- if the array is empty, it returns a blank string). Is this satisfactory to get committed?

Comment #7

joelstein CreditAttribution: joelstein commented 11 November 2009 at 23:07

File	Size
devel.module-d6.patch	375 bytes

Hmm, somehow the d6 patch was messed up.

Comment #8

salvis

he/his

CreditAttribution: salvis commented 11 November 2009 at 23:23

Status:

Needs review

» Reviewed & tested by the community

Yes, this looks good to me.

Comment #9

joelstein CreditAttribution: joelstein commented 26 March 2010 at 21:24

File	Size
devel-6.patch	719 bytes

It's been a few months, but the solution hasn't changed. Here's the patch for D6 again, against dev. Should pass the tests. I'll try to get a D7 version soon, but it's just one line.

Comment #10

joelstein CreditAttribution: joelstein commented 26 March 2010 at 21:34

Version:

6.x-1.x-dev

» 7.x-1.x-dev

File	Size
devel-7.patch	719 bytes

Here's the D7 patch.

Comment #11

salvis

he/his

CreditAttribution: salvis commented 27 March 2010 at 13:00

@moshe: Do you see a problem with this?

Comment #12

moshe weitzman CreditAttribution: moshe weitzman commented 27 March 2010 at 14:00

fine with me

Comment #13

salvis

he/his

CreditAttribution: salvis commented 28 March 2010 at 15:19

Status:

Reviewed & tested by the community

» Fixed

Committed to D7 and D6.

@moshe: Ok, thanks. I took #12 as permission to commit. You asked me not to commit things outside of DNA, so I've waited for you to do it. Was there a misunderstanding?

@joelstein: Thanks for your patience. You uploaded the D7 patch (against revision 1.411) twice, but it applied to D6 with some fuzz.

Comment #14

moshe weitzman CreditAttribution: moshe weitzman commented 28 March 2010 at 17:43

@salvis. thanks for committing ... you are welcome to commit bug fixes to any part of devel package. significant new features should go through issue queue review.