Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix issue 254: Make identifier nullable on product variant and order line #1256

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
use Lunar\Base\Migration;

class MakeIdentifierNullableOnOrderLines extends Migration
{
public function up()
{
Schema::table($this->prefix.'order_lines', function (Blueprint $table) {
$table->string('identifier')->nullable()->change();
});
}

public function down()
{
Schema::table($this->prefix.'order_lines', function (Blueprint $table) {
$table->string('identifier')->nullable(false)->change();
});
}
}
2 changes: 1 addition & 1 deletion packages/core/src/Base/Purchasable.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function getOption();
/**
* Return a unique string which identifies the purchasable item.
*
* @return string
* @return ?string
*/
public function getIdentifier();

Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/Models/ProductVariant.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,10 @@ public function getOptions()
*/
public function getIdentifier()
{
return $this->sku;
// TODO: It is better if this is coupled somehow to the config from the admin package
// There product identifiers are set and we cen return the one specified there
// Which will be more fool proof than this workaround
return $this->sku ?? $this->gtin ?? $this->mpn ?? $this->ean ?? null;
}

public function images()
Expand Down
96 changes: 96 additions & 0 deletions packages/core/tests/Unit/Actions/Carts/CreateOrderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Lunar\Tests\Unit\Actions\Carts;

use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Config;
use Lunar\Actions\Carts\CreateOrder;
use Lunar\DataTypes\Price as PriceDataType;
use Lunar\DataTypes\ShippingOption;
Expand Down Expand Up @@ -744,4 +745,99 @@ public function can_create_order_with_customer()

$this->assertDatabaseHas((new Order())->getTable(), $datacheck);
}

/** @test */
public function it_should_create_order_when_no_identifier_is_set_on_product_variant()
{
CustomerGroup::factory()->create([
'default' => true,
]);

$billing = CartAddress::factory()->make([
'type' => 'billing',
'country_id' => Country::factory(),
'first_name' => 'Santa',
'line_one' => '123 Elf Road',
'city' => 'Lapland',
'postcode' => 'BILL',
]);

$shipping = CartAddress::factory()->make([
'type' => 'shipping',
'country_id' => Country::factory(),
'first_name' => 'Santa',
'line_one' => '123 Elf Road',
'city' => 'Lapland',
'postcode' => 'SHIPP',
]);

$taxClass = TaxClass::factory()->create();

$currency = Currency::factory()->create([
'decimal_places' => 2,
]);

$cart = Cart::factory()->create([
'currency_id' => $currency->id,
]);

$taxClass = TaxClass::factory()->create([
'name' => 'Foobar',
]);

$taxClass->taxRateAmounts()->create(
TaxRateAmount::factory()->make([
'percentage' => 20,
'tax_class_id' => $taxClass->id,
])->toArray()
);

$purchasable = ProductVariant::factory()->create([
'tax_class_id' => $taxClass->id,
'unit_quantity' => 1,
'sku' => null,
'gtin' => null,
'mpn' => null,
'ean' => null,
]);

Price::factory()->create([
'price' => 100,
'tier' => 1,
'currency_id' => $currency->id,
'priceable_type' => get_class($purchasable),
'priceable_id' => $purchasable->id,
]);

$cart->lines()->create([
'purchasable_type' => get_class($purchasable),
'purchasable_id' => $purchasable->id,
'quantity' => 1,
]);

$cart->addresses()->createMany([
$billing->toArray(),
$shipping->toArray(),
]);

$shippingOption = new ShippingOption(
name: 'Basic Delivery',
description: 'Basic Delivery',
identifier: 'BASDEL',
price: new PriceDataType(500, $cart->currency, 1),
taxClass: $taxClass
);

ShippingManifest::addOption($shippingOption);

$cart->shippingAddress->update([
'shipping_option' => $shippingOption->getIdentifier(),
]);

$cart->shippingAddress->shippingOption = $shippingOption;

$order = $cart->createOrder();

$this->assertNull($order->lines->first()->identifier);
}
}
80 changes: 80 additions & 0 deletions packages/core/tests/Unit/Models/ProductVariantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,84 @@ public function can_get_correct_price_inc_tax_based_on_tax_class()
$this->assertEquals(416, $foodProductVariant->pricing()->currency($currency)->get()->matched->priceIncTax()->value);
$this->assertEquals(9760, $genericProductVariant->pricing()->qty(20)->currency($currency)->get()->matched->priceIncTax()->value);
}

/** @test */
public function it_should_use_gtin_when_sku_is_null()
{
// Given
$product = Product::factory()->create();
$gtin = '123';
$productVariant = ProductVariant::factory()->create([
'product_id' => $product->id,
'sku' => null,
'gtin' => $gtin
]);

// When
$identifier = $productVariant->getIdentifier();

// Then
$this->assertEquals($gtin, $identifier);
}

/** @test */
public function it_should_use_mpn_when_sku_and_gtin_are_null()
{
// Given
$product = Product::factory()->create();
$mpn = '123';
$productVariant = ProductVariant::factory()->create([
'product_id' => $product->id,
'sku' => null,
'gtin' => null,
'mpn' => $mpn
]);

// When
$identifier = $productVariant->getIdentifier();

// Then
$this->assertEquals($mpn, $identifier);
}

/** @test */
public function it_should_use_ean_when_sku_gtin_and_mpn_are_null()
{
// Given
$product = Product::factory()->create();
$ean = '123';
$productVariant = ProductVariant::factory()->create([
'product_id' => $product->id,
'sku' => null,
'gtin' => null,
'mpn' => null,
'ean' => $ean
]);

// When
$identifier = $productVariant->getIdentifier();

// Then
$this->assertEquals($ean, $identifier);
}

/** @test */
public function it_should_return_null_when_all_identifiers_are_null()
{
// Given
$product = Product::factory()->create();
$productVariant = ProductVariant::factory()->create([
'product_id' => $product->id,
'sku' => null,
'gtin' => null,
'mpn' => null,
'ean' => null
]);

// When
$identifier = $productVariant->getIdentifier();

// Then
$this->assertEquals(null, $identifier);
}
}