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

$quantity and $unitPriceOverrides in TransactionNonCatalogPrice class should be optional/nullable? #87

Closed
dutypro opened this issue Oct 4, 2024 · 8 comments
Assignees

Comments

@dutypro
Copy link
Contributor

dutypro commented Oct 4, 2024

Describe the bug

As per the docs for previewing a transaction: https://developer.paddle.com/api-reference/transactions/preview-transaction

Both price.unit_price_overrides and price.quantity are optional on the 'Non catalog price for an existing product' item requirements.

The TransactionNonCatalogPrice doesn't allow null for the constructor signatures. Same for customData.

Should they be nullable to allow omission (and adopt default values for price.quantity)?

If so, let me know your thoughts and I'll submit a PR. I'm going to be working on the 'Non-catalog price and product' workflow tomorrow so if I find the same, I'll lump it all in to one PR.

Steps to reproduce

Entities > Transaction > TransactionNonCatalogPrice.php

Expected behavior

Allow unitPriceOverrides, quantity and customData to be null as they are optional.

Code snippets

class TransactionNonCatalogPrice
{
    /**
     * @param array<UnitPriceOverride> $unitPriceOverrides
     */
    public function __construct(
        public string $description,
        public string|null $name,
        public TimePeriod|null $billingCycle,
        public TimePeriod|null $trialPeriod,
        public TaxMode $taxMode,
        public Money $unitPrice,
        public array $unitPriceOverrides,
        public PriceQuantity $quantity,
        public CustomData|null $customData,
        public string $productId,
    ) {
    }
}

// I think it should be (keys still need to be passed as API requires them):

class TransactionNonCatalogPrice
{
    /**
     * @param array<UnitPriceOverride> $unitPriceOverrides
     */
    public function __construct(
        public string $description,
        public string|null $name,
        public TimePeriod|null $billingCycle,
        public TimePeriod|null $trialPeriod,
        public TaxMode $taxMode,
        public Money $unitPrice,
        public array|null $unitPriceOverrides,
        public PriceQuantity|null $quantity,
        public CustomData|null $customData,
        public string $productId,
    ) {
    }
}

PHP version

8.3.10

SDK version

1.3.1

API version

1

Additional context

If you agree it's a bug, I'll submit a PR in the next couple of days.

@dutypro dutypro changed the title $quantity and $unitPriceOverrides in TransactionNonCatalogPrice class should be optional? $quantity and $unitPriceOverrides in TransactionNonCatalogPrice class should be optional/nullable? Oct 4, 2024
@dutypro
Copy link
Contributor Author

dutypro commented Oct 4, 2024

Can I just clarify something...

If I am submitting a Transaction Preview with a non-catalog price for an existing product, does Paddle on the back end create a price object and therefore a price_id for it?

It's my understanding from the docs that it doesn't need to as a transaction hasn't been created.

@dutypro
Copy link
Contributor Author

dutypro commented Oct 4, 2024

Apologies, adding comments as notes.

It doesn't appear to create a price object but it does generate a price_id.

See #88

@davidgrayston-paddle
Copy link
Contributor

Hello @dutypro 👋

Thank you for reporting this issue, you are correct that these properties should be optional. The fix will involve creating new request operation objects with optional properties, so we will provide a fix for this shortly.

Regarding question:

If I am submitting a Transaction Preview with a non-catalog price for an existing product, does Paddle on the back end create a price object and therefore a price_id for it?

A price will not be created, and the price ID in the response will be null – this will be fixed in the SDK at the same time

@dutypro
Copy link
Contributor Author

dutypro commented Oct 4, 2024

Hello @dutypro 👋

Thank you for reporting this issue, you are correct that these properties should be optional. The fix will involve creating new request operation objects with optional properties, so we will provide a fix for this shortly.

Regarding question:

If I am submitting a Transaction Preview with a non-catalog price for an existing product, does Paddle on the back end create a price object and therefore a price_id for it?

A price will not be created, and the price ID in the response will be null – this will be fixed in the SDK at the same time

Thanks David. You pipped me to the post as I was creating a bug on #88 but thought I'd submit it anyway as I'd typed it all out!

@davidgrayston-paddle
Copy link
Contributor

Thank you for reporting these issues @dutypro - we'll address both at the same time

@dutypro
Copy link
Contributor Author

dutypro commented Oct 4, 2024

Thanks David.

It was a blocker for me so as temp workaround locally, I've allowed id to be null in the Price constructor:

public ?string $id,

And everything seems to be now working as expected.

@dutypro
Copy link
Contributor Author

dutypro commented Oct 6, 2024

Thanks David.

It was a blocker for me so as temp workaround locally, I've allowed id to be null in the Price constructor:

public ?string $id,

And everything seems to be now working as expected.

I've tested the 'Non-catalog price and product' workflow and the same issues arise for $productId in the Price constructor and $id in Product constructor.

@davidgrayston-paddle
Copy link
Contributor

This has now been fixed and included 1.4.0 release, with a summary of the related changes included in the changelog

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

No branches or pull requests

2 participants