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

Add price override in react delivery form #4860

Open
wants to merge 42 commits into
base: master
Choose a base branch
from

Conversation

CamilleNerriere
Copy link
Member

@CamilleNerriere CamilleNerriere commented Jan 24, 2025

  • Add override Price and restriction for admin
  • Create a widget to convert price with/without VAT
  • Fix issues with Calculate prices
  • Change range for timeslots in DateRangePicker (10 minutes)
  • Add autofill address if activated
  • Add multidropoff only if activated
  • UX : make all task header clicable to toggle

#4851
#4848
#4853
#4854
#4856

r0xsh and others added 30 commits January 21, 2025 16:12
```javascript
fetch('/api/task?groups=task,address,barcode')
  .then((response) => response.json())
  .then((data) => console.log(data));

```
@Atala Atala requested review from alexsegura and removed request for Atala January 30, 2025 15:36
@@ -49,11 +49,16 @@ final class TaxRate

public function __construct(BaseTaxRate $taxRate, string $name, array $alternatives = [])
{
var_dump($taxRate->getCode());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed

* @var DeliveryPriceInput
* @Groups({"delivery_create"})
*/
private $deliveryPriceInput;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK there is no need to create a new DeliveryPriceInput for this, you can use the existing ArbitraryPrice.
This way in Appbundle\Action\Delivery\Create, you don't need to convert the DeliveryPriceInput into an ArbitraryPrice. Just put the @var ArbitraryPrice and/or add type hinting, and API Platform will understand.

Comment on lines +34 to +37
$arbitraryPrice = new ArbitraryPrice(
$data->getDeliveryPriceInput()->getVariantName(),
$data->getDeliveryPriceInput()->getPriceIncVATcents()
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other comment, you could avoid doing this conversion by using ArbitraryPrice for serialization.

@@ -26,7 +28,20 @@ public function __invoke(Delivery $data)
throw new ValidationException($errors);
}

$this->pricingManager->createOrder($data);
$useArbitraryPrice = $this->authorizationCheckerInterface->isGranted('ROLE_ADMIN') && !is_null($data->getDeliveryPriceInput());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use ArbitraryPrice in Delivery class, !is_null($data->getDeliveryPriceInput()) could be replaced by a more expressive & positive method name, hasArbitraryPrice().

Comment on lines +2476 to +2479
"deliveryPriceInput": {
"priceIncVATcents": 1200,
"variantName": "my custom variant"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong here. To test that the order has been created, yes you should target the orders API.
Eventually, you can return an IRI corresponding to the created order:

"order": "/api/orders/1"

Comment on lines +2491 to +2495
"deliveryPrice": {
"variantName": "my custom variant",
"value": 1200,
"@*@": "@*@"
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this, you just need to check the value of total

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wanted to check the variant name, how can i do that without this call? (+ I need the variant name when editing the delivery)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the field in OrderItemNormalizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants