[1.0.x] Commerce Limit Subscriptions

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications → , What to cover in an application review → , and Drupal.org security advisory coverage application workflow → .

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

@kishan@lnwebworks Remember to change the status to Needs review when the project is ready for review.

Needs Review

Remove the README.txt file since the README.md file is already present.

Removed the README.txt file

Rest looks fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

src/EventSubscriber/LimitSubscriptions.php

/**
 * Provides the default LimitSubscriptions.
 *
 * @package Drupal\commerce_limit_subscriptions
 */
class LimitSubscriptions implements EventSubscriberInterface {

The short description does not explain what its purpose is. For a LimitSubscriptions class Provides the default LimitSubscriptions. does not make much sense.

  /**
   * Constructs a new LimitSubscriptions object.
   *
   * Constructs a new CartController object.

There are two short description. Neither of them uses the class namespace.

   * @param \Drupal\commerce_store\CurrentStoreInterface $current_store
   *   The current store.
   *   Constructs a new AvailabilityChecker object.

The last line is extraneous.

  /**
   * Limit subscriptions function called on add to cart event.
   */
  public function limitSubscriptions(CartEntityAddEvent $event) {

The documentation comment does not describe the method parameters.

$this->messenger->addError("You have already added subscriptions product in cart");

Messages passed to the messenger needs to be translatable strings.
subscriptions product is probably supposed to be product subscriptions.

There are also many empty lines that need to be removed.

/commerce_limit_subscriptions.module

$output .= '<p>' . t('The Commerce limit Subscriptions module restrict user to buy subscription again if he have already active subscriptions...') . '</p>';

It should be restricts users. (module is singular, not plural.)

Thank you for the feedback.

1. Added short description for explanations.
2. Added class namespace in short description.
3.Removed the extraneous line
4.Removed the empty spaces.
5. described the method parameters in documentation
6.translatable strings added
7.Updated restricts users

Above changes are made in the default branch.

The info.yml file maybe needs some improvemets:

dependencies:
  - drupal:commerce
  - drupal:commerce_recurring

Please check how this works:
https://www.drupal.org/docs/develop/creating-modules/let-drupal-know-abo... →

ependencies: a list of other modules your module depends on. Dependencies on Drupal core or contrib modules should be namespaced in the format {project}:{module}, where {project} is the project name as it appears in the Drupal.org URL (e.g. drupal.org/project/paragraphs) and {module} is the module's machine name.

Thank you for the feedback

dependencies:
- commerce:commerce
- commerce_recurring:commerce_recurring

Make the above changes in info.yml file

Some code enhancements.

in line 154:
if ($has_field && !empty($has_field)) {
the $has_field && part is redundant as it's checked in the second condition.

Same in line 161:
if ($quantity >= 2 && $has_field && !empty($has_field)) {

and 178:
if ($subscription_products_count > 1 && $has_field && !empty($has_field)) {

In line 185:
$this->cartManager->removeOrderItem($order, $order_item);
$order_item may be NULL if $cart->getItems()is null, so WSOD could happen.

In line 199:
$this->messenger->addError($this->t('You have already active subscriptions'));
The message should end with a .

Thank you for the feedback @alvar0hurtad0

1.if ($has_field && !empty($has_field)) {
the $has_field && part is redundant as it's checked in the second condition.

Similar issue are fixed.
2.$order_item may be NULL if $cart->getItems()is null, so WSOD could happen.

Not empty if condition is added for the above.

3. $this->messenger->addError($this->t('You have already active subscriptions'));
The message should end with a .
. is added in the message.

Above issues are fixed .

Thanks for the fixes @kishan@lnwebworks I still see some enhancements:

line 81:

   * @param Drupal\Core\Messenger\MessengerInterface $messenger
   *   The Messenger.

There's a missing \

line 193:

if (!empty($query) && !empty($field)) {
$field variable should not be empty but I can't find the initialisation of $field variable in the method.

  public function limitSubscriptions(CartEntityAddEvent $event) {

    // Entity type manager service.
    // Getting subscription storage.
    $subscription_storage = $this->entityTypeManager->getStorage("commerce_subscription");
    // Getting current user id using service.
    $current_user_id = $this->currentUser->id();
    // Entity query for getting subscriptions ids for subscriptions.
    $query = $subscription_storage->getQuery()->condition("uid", $current_user_id)->condition("state", ["active"], "IN")->accessCheck(FALSE)->execute();
    $store = $this->currentStore->getStore();
    $cart = $this->cartProvider->getCart("default", $store);
    $simple_products_count = 0;
    $subscription_products_count = 0;

    foreach ($cart->getItems() as $order_item) {

      $order = $order_item->getOrder();
      $entity = $order_item->getPurchasedEntity();
      $has_field = $entity->hasField('billing_schedule');
      $quantity = $order_item->quantity->value;

      // Increment the counter for the respective product type.
      if (!empty($has_field)) {
        $subscription_products_count++;
      }
      else {
        $simple_products_count++;
      }

      if ($quantity >= 2 && !empty($has_field)) {

        $matching_order_item = $this->orderItemMatcher->match($order_item, $cart->getItems());
        if ($matching_order_item) {

          $new_quantity = 1;
          $matching_order_item->setQuantity($new_quantity);
          $matching_order_item->save();

        }
        $this->messenger->deleteAll();
        $this->messenger->addError($this->t('You have already added product subscriptions in cart.'));
        $response = new RedirectResponse('/cart');
        $response->send();
        return;
      }
    }
    if ($subscription_products_count > 1 && !empty($has_field)) {
      foreach ($cart->getItems() as $order_item) {
        $this->messenger->deleteAll();
        $this->messenger->addError($this->t('You have already added product subscriptions in cart.'));
      }
      $ord_id = $order->id();
      $order = $this->entityTypeManager->getStorage('commerce_order')->load($ord_id);
      if (!empty($order_item)) {
        $this->cartManager->removeOrderItem($order, $order_item);
        $response = new RedirectResponse('/cart');
        $response->send();
        return;
      }
    }

    if (!empty($query) && !empty($field)) {
      $this->messenger->deleteAll();
      $order_item = $event->getOrderItem();
      $orders = $this->cartProvider->getCarts();

      foreach ($orders as $order) {
        $this->cartManager->removeOrderItem($order, $order_item);
      }
      $this->messenger->addError($this->t('You have already active subscriptions.'));
    }
  }

sorry I forgot to change the status

Thank you for the feedback alvar0hurtad0 →
1.Updated

* @param Drupal\Core\Messenger\MessengerInterface $messenger
   *   The Messenger.
There's a missing \

2.Updated $field variable value

Hello @kishan@lnwebworks,
sorry for the ping-pong but I still see some enhancements:

  /**
   * The current user.
   *
   * @var \Drupal\Core\Session\AccountProxy
   */
  protected $currentUser;

Maybe this should be @var \Drupal\Core\Session\AccountProxyInterface as the rest of attributes.

Thank you for the feedback @ alvar0hurtad0 →

1.  /**
   * The current user.
   *
   * @var \Drupal\Core\Session\AccountProxy
   */
  protected $currentUser;

Updates this as @var \Drupal\Core\Session\AccountProxyInterface

   * Constructs a new LimitSubscriptions object for LimitSubscriptions class.

There is no need to add for LimitSubscriptions class since that is already implicit in Constructs a new LimitSubscriptions object which means Constructs a new instance of the LimitSubscriptions class.

        $response = new RedirectResponse('/cart');
        $response->send();
        return;

I am not sure an event subscribed is supposed to redirect, since that would avoid other event subscribers can react to the same event; it would also interrupt the module that created that event.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries → ' post on Responsible maintainers
• Best practices for creating and maintaining projects →
• Maintaining a drupal.org project with Git →
• Commit messages - providing history and credit →
• Release naming conventions → .
• Helping maintainers in the issue queues →

You can find more contributors chatting on the Slack → #contribute channel. So, come hang out and stay involved → .
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review → . I encourage you to learn more about that process and join the group of reviewers.

I thank all the reviewers.

Automatically closed - issue fixed for 2 weeks with no activity.