I have to extend the wish list with a type, i.e. birthday, wedding, etc. People can specify and change this type when creating a wish list.

The purpose is to allow styling to be done based on the wish list type.

As this seems to me a generically useful feature, are people interested in a patch?

CommentFileSizeAuthor
#11 uc_wishlist-add-wishlist-type-2397457-11.patch4.79 KBberenddeboer
#4 interdiff-3-4.txt1 KBnaveenvalecha
#4 uc_wishlist-add-wishlist-type-2397457-4.patch4.79 KBnaveenvalecha
#3 uc_wishlist-add-wishlist-type-2397457-3.patch4.53 KBberenddeboer
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha commented
Parent issue: » #2386985: Module Release 7.x-1.3

Added parent issue.Will need to discuss about this feature with @Jay and will update here that we are going to add this feature in the module.

berenddeboer’s picture

berenddeboer CreditAttribution: berenddeboer commented
Assigned: Unassigned » berenddeboer
berenddeboer’s picture

berenddeboer CreditAttribution: berenddeboer commented
FileSize
uc_wishlist-add-wishlist-type-2397457-3.patch4.53 KB

Here is the patch: in the settings you can specify types. Only if you have setup types, a user can see/select them. Type is emitted as class in the form. More useful would be to have the class in the body, but that depends on your particular theme or other support modules which may make that easy.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha commented
Status: Active » Needs review
FileSize
uc_wishlist-add-wishlist-type-2397457-4.patch4.79 KB
interdiff-3-4.txt1 KB

Thanks @berenddeboer, for writing up the patch.Rerolled the patch with below changes.
@Jay it seems ready to go.Would you review it because it contains little bit my code as well.

  1. +++ b/uc_wishlist.pages.inc
@@ -223,6 +223,20 @@ function uc_wishlist_settings_form($form, $form_state, $wishlist, $collapsed = F
+      $options[$type] = t($type);

    Need to sanitize the $type variable here using check_plain.One more thing only strict literals should be passed to t as per coding standards. 

  2. +++ b/uc_wishlist.pages.inc
@@ -223,6 +223,20 @@ function uc_wishlist_settings_form($form, $form_state, $wishlist, $collapsed = F
+    $types = $types = explode("\n", $types);

    $types variable two times. Need to remove once

  3. Removed the used variable in hook_uninstall.

Status: Needs review » Needs work

The last submitted patch, 4: uc_wishlist-add-wishlist-type-2397457-4.patch, failed testing.

naveenvalecha queued 4: uc_wishlist-add-wishlist-type-2397457-4.patch for re-testing.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha commented
Version: 7.x-1.2 » 7.x-1.x-dev

Changing the version so that testbot will take a look at it.

The last submitted patch, 4: uc_wishlist-add-wishlist-type-2397457-4.patch, failed testing.

naveenvalecha queued 4: uc_wishlist-add-wishlist-type-2397457-4.patch for re-testing.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha commented
Status: Needs work » Needs review
berenddeboer’s picture

berenddeboer CreditAttribution: berenddeboer commented
FileSize
uc_wishlist-add-wishlist-type-2397457-11.patch4.79 KB
berenddeboer’s picture

berenddeboer CreditAttribution: berenddeboer commented

Thanks for the quick review, and catching my mistakes, always a pleasure working with you!

Have attached one more fix: if a user enters a space on a line, you get an option with a space, and Drupal doesn't like that, you get "illegal choice detected" so added a trim.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Status: Needs review » Needs work
Issue tags: +Needs reroll