shortcut_set_unassign_user deletes in the wrong table [#1228888]

While removing a user from a shortcut set in a module I was making, I received an error about column UID not being found. Looking in the shortcut.module I saw that shortcut_set_unassign_user was using the wrong table to remove the user from the shortcut set.

Attached patch correctly unassigns a user from the shortcut_set.

This is the first time that I'm submitting a patch to drupal core so forgive my ignorance if I'm doing this wrong.

The patch is made against drupal 7.4

Comment	File	Size	Author
#11	shortcut_unassign-1228888-4778138.patch	1.39 KB	Johnny vd Laar
#11
#3	shortcut_unassign-1228888-4777606.patch	512 bytes	Johnny vd Laar
#3
	shortcut_unassign_bug.patch	484 bytes	Johnny vd Laar

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

25 July 2011 at 14:42

Status:

Needs review

» Needs work

The last submitted patch, shortcut_unassign_bug.patch, failed testing.

Comment #2

Johnny vd Laar CreditAttribution: Johnny vd Laar commented 25 July 2011 at 14:50

ok guess it was meant to fail at first try. going to try to create a new patch hold on ;-)

Comment #3

Johnny vd Laar CreditAttribution: Johnny vd Laar commented 25 July 2011 at 14:59

Version:

7.4

» 7.x-dev

File	Size
shortcut_unassign-1228888-4777606.patch	512 bytes

ok second attempt made this to latest 7.x-dev version from git and created the patch with git instead of svn

Comment #4

marcingy CreditAttribution: marcingy commented 25 July 2011 at 15:13

Version:	7.x-dev	» 8.x-dev
Issue tags:		+Needs tests, +Needs backport to D7

This needs a test. And also moving to d8.

Comment #5

marcingy CreditAttribution: marcingy commented 25 July 2011 at 15:19

Status:

Needs work

» Needs review

Setting to needs review for the bot.

Comment #6

Johnny vd Laar CreditAttribution: Johnny vd Laar commented 25 July 2011 at 15:22

ah thank you I already wondered why it didn't test anymore

Comment #7

Johnny vd Laar CreditAttribution: Johnny vd Laar commented 25 July 2011 at 16:02

Yay, so is there anything I'm supposed to do now?

Comment #8

marcingy CreditAttribution: marcingy commented 25 July 2011 at 16:04

Status:

Needs review

» Needs work

We need a simpletest to test the behaviour is correct when shortcuts are deleted.

Comment #9

Johnny vd Laar CreditAttribution: Johnny vd Laar commented 25 July 2011 at 16:30

Well the function is never used by drupal core itself:
http://api.drupal.org/api/drupal/modules--shortcut--shortcut.module/func...

So will it be sufficient to add this?


  /**
   * Tests unassigning a shortcut set.
   */
  function testShortcutSetUnassign() {
    $new_set = $this->generateShortcutSet($this->randomName(10));

    shortcut_set_assign_user($new_set, $this->shortcut_user);
    shortcut_set_unassign_user($this->shortcut_user);
    $current_set = shortcut_current_displayed_set($this->shortcut_user);
    $default_set = shortcut_default_set($this->shortcut_user);
    $this->assertTrue($current_set->set_name == $default_set->set_name, "Successfully unassigned another user's shortcut set.");
  }