Each wishlist item has a quantity set by the customer.

We need to know the following:
- When the item was last purchased
- How much was purchased each time (to be able to remove the item when fully bought, for example)

My initial idea was to add purchased_quantity and last_purchased fields to the wishlist item.
But this becomes a problem when considering the "Don't spoil my surprises" option, which hides a purchase for a few weeks.
In that case you want to hide only a specific quantity purchase until it's safe to reveal it.
So it's best to have a multivalue field that stores order_id, quantity, purchased (timestamp).

Comments

bojanz created an issue. See original summary.

czigor’s picture

Status: Active » Needs review
StatusFileSize
new7.58 KB

Adding a field type and a formatter. As discussed on slack, no widget.

Status: Needs review » Needs work

The last submitted patch, 2: commerce_wishlist-3025019-purchase_field-2.patch, failed testing. View results

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new9.99 KB
bojanz’s picture

Status: Needs review » Needs work

The field instance needs to be called "purchases", with getPurchases() setPurchases() and addPurchase().

We should create a Purchase value object that has getOrderId() getQuantity() getPurchasedTime() methods, and is used by the getters/setters.

WishlistPurchasedFormatter should be WishlistPurchaseFormatter and it should output a table like the order item formatter.

+    $columns['purchased'] = [

Unsure about the column name. "timestamp" might be better. Ask Matt/Jonathan for a second opinion.

czigor’s picture

StatusFileSize
new14.15 KB

Fixed all the above. I went with 'purchased_time' for the timestamp column. Still unsure about it, 'purchase_time' might actually be better.

czigor’s picture

Status: Needs work » Needs review
czigor’s picture

Matt votes for purchased_time.

agoradesign’s picture

StatusFileSize
new13.84 KB
new2.26 KB

Here's an updated patch, that does two things:

  • fixes the typo "purhcased"
  • Simplifies the setValue() function of the field type by calling the value object's toArray()

Status: Needs review » Needs work

The last submitted patch, 9: commerce_wishlist-3025019-purchase_field-9.patch, failed testing. View results

agoradesign’s picture

ohhh

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new14.33 KB
new759 bytes

Fixing update hook function names.

agoradesign’s picture

StatusFileSize
new13.8 KB
new3.27 KB

wow... you were incredibly quick.. but I think, this is one is more correct... b/c the other two hooks are already committed

czigor’s picture

Ah yes, i was just fixing the applied patch without thinking. Looks good to me.

agoradesign’s picture

same to me before when fixing the typo :D

bojanz’s picture

Status: Needs review » Needs work
+function commerce_wishlist_update_8304() {
+  drupal_flush_all_caches();

Why do we need to flush caches at the beginning of this update function?
An irregular thing like that needs a comment with an explanation, assuming that it's actually needed.

+  public function getPurchases() {
+    return $this->get('purchases')->getValue();
+  }

Docs say that getPurchases will return an array of value objects. getValue() does not return an array of value objects.

Which brings me to tests. We need a unit test for the value object. A kernel test for wishlist items that will test the purchases methods.

+  public function __toString() {
+    $this->stringTranslation = \Drupal::service('string_translation');
+    /** @var \Drupal\Core\Datetime\DateFormatterInterface $date_formatter */
+    $date_formatter = \Drupal::service('date.formatter');
+    $vars = [
+      '@purchased' => $date_formatter->format($this->purchasedTime),
+      '@order_id' => $this->orderId,
+    ];
+    return $this->formatPlural($this->quantity, '@count item has been bought in order @order_id on @purchased.', '@count items have been bought in order @order_id on @purchased.', $vars);
+  }

Using \Drupal in a value object is icky. A __toString() method is meant for debugging purposes, not for returning a string that can be shown in a UI. I'd remove the entire __toString() method.

+  public function viewElements(FieldItemListInterface $items, $langcode) {
+    $elements = [
+      '#type' => 'table',
+      '#caption' => $this->t('Purchased items'),

Caption is incorrect. We're listing Purchases, not purchased items.

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new24 KB

Replaced drupal_flush_all_caches() with clearing only the field type cache.

Added the kernel and unit tests and fixed the other points too.

Status: Needs review » Needs work

The last submitted patch, 17: commerce_wishlist-3025019-purchase_field-17.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new24.37 KB

Fixing coding standards.

Status: Needs review » Needs work

The last submitted patch, 19: commerce_wishlist-3025019-purchase_field-19.patch, failed testing. View results

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new25.36 KB

Use dev from DC core to let the trait be found.

Status: Needs review » Needs work

The last submitted patch, 21: commerce_wishlist-3025019-purchase_field-21.patch, failed testing. View results

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new24.74 KB

Status: Needs review » Needs work

The last submitted patch, 23: commerce_wishlist-3025019-purchase_field-23.patch, failed testing. View results

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new24.74 KB

Try again.

  • bojanz committed bd536f0 on 8.x-3.x authored by czigor
    Issue #3025019 by czigor, agoradesign, bojanz: Create a...
bojanz’s picture

Status: Needs review » Fixed

Sneaked in getPurchasedQuantity() and getLastPurchasedTime() order item methods.

Reverted purchased -> purchased_time rename, the original made more sense.
Fixed bug that was causing the Reports screen to always report the field as needing an update (due to the current time default value for "placed").
Other cleanups.

Status: Fixed » Closed (fixed)

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