Always load ALL redemptions?

[d--j]

Hello,

Do I understand

gift-voucher/src/services/Redemptions.php

Lines 38 to 46 in 3a20c0b

	public function getRedemptionById(int $id): ?Redemption
	{
	return $this->_redemptions()->firstWhere('id', $id);
	}
	public function getRedemptionsByCodeId(int $codeId): array
	{
	return $this->_redemptions()->where('codeId', $codeId)->all();
	}

gift-voucher/src/services/Redemptions.php

Lines 125 to 153 in 3a20c0b

	private function _redemptions(): MemoizableArray
	{
	if (!isset($this->_redemptions)) {
	$redemptions = [];
	foreach ($this->_createRedemptionsQuery()->all() as $result) {
	$redemptions[] = new Redemption($result);
	}
	$this->_redemptions = new MemoizableArray($redemptions);
	}
	return $this->_redemptions;
	}
	private function _createRedemptionsQuery(): Query
	{
	return (new Query())
	->select([
	'id',
	'codeId',
	'orderId',
	'amount',
	'dateCreated',
	'dateUpdated',
	'uid',
	])
	->from(['{{%giftvoucher_redemptions}}']);
	}

correctly in that you will load ALL redemptions when searching for a single redemption or a redemption for a specific code?

Why did you opt to memoize the database queries for redemptions? I have searched for usage of getRedemptionById/getRedemptionsByCodeId/deleteRedemptionById but did not find any that would warrant memoization in my opinion.

Having someday hopefully millions of redemptions in the database, this looks strange to me.

Side note: the return value of deleteRedemptionById / deleteRedemption might not be very meaningful right now. deleteRedemption always returns true – so why return something? deleteRedemptionById will return false when it initially does not find the ID – but when you call it multiple times with the same $id it will return true every time. It should return true only the first time.

[engram-design]

I'll agree that this probably isn't the right approach for this sort of data, but the goal was performance gains. But as you mention, that's going to do the opposite, if it isn't already for large installs.

For the delete actions, calling deleteRedemptionById doesn't return true every time with the same ID. That's because it should be deleted the first time called, and the next call will fail trying to call getRedemptionById.

As for why deleteRedemption always returns true, if we're wanting to be pedantic about the return value, I can change that to factor in any delete rows made by that Db::delete command.

To get this early, run:

• composer require verbb/gift-voucher:"dev-craft-4 as 3.0.14"
• composer require verbb/gift-voucher:"dev-craft-5 as 4.0.4"