Goals and Implementation overview

Phase 3:

  1. Option to email wishlist to others
    • This feature would enable users or wish list owners to share any wish list with other users or potential customers by emailing it to the respective user.
    • The following email properties are defined in an appropriate​ hook_mail implementation:
      • Email subject: Includes a brief synopsis of the wish list contents.
      • Email recipients: Includes the email addresses of other users to whom the wishlist is concerned.
      • Email message: A text area which includes any necessary message to be accompanied with the concerned wishlist.
    • The Form API and Database API would be used for the creation, submission & validation of the wish list email and to provide a structured interface for the dynamic construction of queries to implement the above functionality.@naveenvalecha,

      Fresh patch contains port of 'Add User wishlist settings' along with 'email wish list' and
      'add user wishlist settings' screenshots.

      Please review.

    • Conversion of D7 Hook functions to D8 APIs involving Routes, to define the path to Controllers (page callbacks in D7) or to create tabs and/or contextual links. Email address of an user is verified through the Data Common API.
    • The Renderable Array System has to be applied to improve the grouping of form items. The email could be sent by using Mail Manager using public function MailManager::mail
  2. Add User wish list settings
    • This feature is implemented by creating a table for the required wish list settings using the Form and Database API
    • The mandatory fields, including User, Title, Expiration Date & Status are created using Field API and the Hook functions hook_field_settings_form & hook_field_widget_form. The Configuration API would be used to configure the fields.
    • db_select is implemented with a Join to join/combine with another table containing relevant information.
    • The Renderable Array system would be applied to improve the grouping of the contents. The Field Theme System would be configured.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chiranjeeb2410 created an issue. See original summary.

chiranjeeb2410’s picture

Issue summary: View changes
naveenvalecha’s picture

Project: Uc Wishlist » UC Wish List
Version: » 8.x-1.x-dev
chiranjeeb2410’s picture

Title: Option to email wishlist to others » Porting Uc wishlist module to Drupal 8 | Phase 3
Issue summary: View changes
chiranjeeb2410’s picture

Issue summary: View changes
chiranjeeb2410’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
92.2 KB

@naveenvalecha,

Uploaded fresh patch for final port. Includes features to 'Add user wish list settings'
and 'Email wish list to others'.

Please review.

naveenvalecha’s picture

Status: Needs review » Needs work

This is the wrong patch. See how to create a patch https://www.drupal.org/node/707484

chiranjeeb2410’s picture

@naveenvalecha,

Working on it.

chiranjeeb2410’s picture

Status: Needs work » Needs review
FileSize
92.85 KB

@naveenvalecha,

Uploaded new patch. Please review.

naveenvalecha’s picture

Status: Needs review » Needs work

This is the wrong patch. See how to create a patch https://www.drupal.org/node/707484

chiranjeeb2410’s picture

@naveenvalecha,

Is the patch not getting applied to your local?

naveenvalecha’s picture

The patch is getting applied but at the wrong place. Read this carefully https://www.drupal.org/node/707484

chiranjeeb2410’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
11.14 KB
127.57 KB
113.25 KB
chiranjeeb2410’s picture

Fixed coding standards and made minor improvements.

naveenvalecha’s picture

Status: Needs review » Needs work
  1. +++ b/src/Form/WishlistSettingsForm.php
    @@ -0,0 +1,194 @@
    + * @file contains user wishlist settings.
    

    Improve the comment.

  2. +++ b/src/Form/WishlistSettingsForm.php
    @@ -0,0 +1,194 @@
    +class WishlistSettingsForm extends ConfigFormBase {
    

    If this is User Wishlist Settings form then rename the class name which reflects its purpose.

  3. +++ b/src/Form/WishlistSettingsForm.php
    @@ -0,0 +1,194 @@
    +    $account = \Drupal::currentUser();
    

    inject it with DI

  4. +++ b/uc_wishlist.links.menu.yml
    @@ -1,4 +1,4 @@
    +  uc_wishlist.settings:
    

    unrelated change

  5. +++ b/uc_wishlist.links.tasks.yml
    @@ -1,5 +1,12 @@
    +uc_wishlist.tab_2:
    

    rename the tab_2 to wishlist_email or something more meaningful

  6. +++ b/uc_wishlist.routing.yml
    @@ -67,10 +67,30 @@ uc_wishlist.wishlist.search:
    +    _permission: 'Administer wish list settings.'
    

    Wrong permission name. Have you tested your code?

chiranjeeb2410’s picture

@naveenvalecha,

Working on it.

chiranjeeb2410’s picture

Status: Needs work » Needs review
FileSize
12.04 KB

@naveenvalecha,

Added required changes.

Please review.

chiranjeeb2410’s picture

Added corrections to route file.

chiranjeeb2410’s picture

FileSize
127.57 KB

@naveenvalecha,

Screenshots attached of the latest ports. Seem to work as planned.

naveenvalecha’s picture

Status: Needs review » Needs work

could you provide Interdiff please https://www.drupal.org/documentation/git/interdiff

chiranjeeb2410’s picture

Status: Needs work » Needs review
FileSize
113.25 KB
127.57 KB
9.5 KB

Uploaded interdiff displaying latest changes. Also, attached screenshots of the attached ports.
Seem to work as planned.

naveenvalecha’s picture

Status: Needs review » Needs work

Wrong inter diff attached in #21. It's not useful :( Re-read the attached d.org handbook page.

  1. +++ b/src/Form/UserWishlistSettingsForm.php
    @@ -0,0 +1,218 @@
    + * @file contains user wishlist settings to extend the
    

    Remove @file from here.

  2. +++ b/src/Form/UserWishlistSettingsForm.php
    @@ -0,0 +1,218 @@
    +    $config = $this->config('uc_product.settings');
    

    is this uc_product settings. Could you rename this variable to reflect what it is

  3. +++ b/src/Form/UserWishlistSettingsForm.php
    @@ -0,0 +1,218 @@
    +    $account = \Drupal::currentUser();
    

    DI the current user service

  4. +++ b/src/Form/UserWishlistSettingsForm.php
    @@ -0,0 +1,218 @@
    +    $account = \Drupal::currentUser();
    

    Inject the current user service

  5. +++ b/uc_wishlist.routing.yml
    @@ -67,10 +67,29 @@ uc_wishlist.wishlist.search:
    +    _permission: 'Administer user wishlist settings.'
    

    This seems not to be the correct permission.

chiranjeeb2410’s picture

@naveenvalecha,

Implementing DI seems to throw some exception errors and doesnt display the form.

What could be the problem?

naveenvalecha’s picture

@chiranjeeb2410,

Implementing DI seems to throw some exception errors and doesnt display the form.

What could be the problem?

Share the error, How could I imagine which problem are you getting.
Read about the Dependency injection:

  1. https://www.drupal.org/docs/8/api/services-and-dependency-injection/serv...
  2. https://www.drupal.org/docs/8/api/services-and-dependency-injection/depe...
chiranjeeb2410’s picture

Working on it.

chiranjeeb2410’s picture

Status: Needs work » Needs review

@naveenvalecha,

This seems to be the error displayed on clearing caches.

PHP Fatal error: Declaration of Drupal\uc_wishlist\Form\UserWishlistSettingsForm::create(Drupal\uc_wishlist\Form\ContainerInterface $container) must be compatible with Drupal\Core\DependencyInjection\ContainerInjectionInterface::create(Symfony\Component\DependencyInjection\ContainerInterface $container) in /var/www/html/drupal/modules/uc_wishlist/src/Form/UserWishlistSettingsForm.php on line 216

However, working on it and will implement DI and post a fresh patch asap.

naveenvalecha’s picture

Related issues: +#2904915: UC Wishlist review

#26,
Could you share the code so that I could reproduce it on my local?


I have reviewed the current 8.x-1.x and tested the functionality of the module on my local and the module is throwing the error. See the review points here #2904915: UC Wishlist review I have marked #2904915: UC Wishlist review as major because it's required for the alpha release of the module and the deliverables of phase 2 and phase 3.

chiranjeeb2410’s picture

@naveenvalecha,

Working on it. Saw the review points. Would be posting a patch for the fixes mentioned in #2904915: UC Wishlist review.

naveenvalecha’s picture

Status: Needs review » Needs work

Setting to correct status.