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
Comment #2
czigor commentedAdding a field type and a formatter. As discussed on slack, no widget.
Comment #4
czigor commentedComment #5
bojanz commentedThe 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.
Unsure about the column name. "timestamp" might be better. Ask Matt/Jonathan for a second opinion.
Comment #6
czigor commentedFixed all the above. I went with 'purchased_time' for the timestamp column. Still unsure about it, 'purchase_time' might actually be better.
Comment #7
czigor commentedComment #8
czigor commentedMatt votes for purchased_time.
Comment #9
agoradesign commentedHere's an updated patch, that does two things:
Comment #11
agoradesign commentedohhh
Comment #12
czigor commentedFixing update hook function names.
Comment #13
agoradesign commentedwow... you were incredibly quick.. but I think, this is one is more correct... b/c the other two hooks are already committed
Comment #14
czigor commentedAh yes, i was just fixing the applied patch without thinking. Looks good to me.
Comment #15
agoradesign commentedsame to me before when fixing the typo :D
Comment #16
bojanz commentedWhy 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.
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.
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.
Caption is incorrect. We're listing Purchases, not purchased items.
Comment #17
czigor commentedReplaced drupal_flush_all_caches() with clearing only the field type cache.
Added the kernel and unit tests and fixed the other points too.
Comment #19
czigor commentedFixing coding standards.
Comment #21
czigor commentedUse dev from DC core to let the trait be found.
Comment #23
czigor commentedComment #25
czigor commentedTry again.
Comment #27
bojanz commentedSneaked 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.