Skip to content

fix(transactions): resolve order URI via connection in create (#16) - #26

Merged
aaronelliotross merged 1 commit into
WeMoveEU:mainfrom
DevMubarak1:fix/transactions-create-order-id
Oct 7, 2026
Merged

aaronelliotross merged 1 commit into
WeMoveEU:mainfrom
DevMubarak1:fix/transactions-create-order-id

Conversation

@DevMubarak1

Copy link
Copy Markdown
Contributor

Fixes #16

The defect

Transactions.create() called Orders.get_resource_uri_from_id(order_id) directly on the Orders class rather than on an instance or via the underlying connection. Because HttpMethods.get_resource_uri_from_id(self, resource_id) is an instance method, calling it unbound bound order_id to self and left resource_id unfilled, raising:

TypeError: get_resource_uri_from_id() missing 1 required positional argument: 'resource_id'

Any call to Transactions.create() providing order_id instead of order_uri crashed before reaching the network.

Fix

Call self.connection.get_resource_uri_from_id(order_id, Orders.resource_name) to build the order resource URI.

Verification

Updated tests/test_transactions.py::test_create_with_order_id_is_broken (renamed to test_create_with_order_id_sends_expected_payload) to assert successful POST dispatch with the generated order URI. All 11 transaction tests pass.

In Transactions.create(), calling Orders.get_resource_uri_from_id(order_id) unbound on the class passed order_id as self, leaving resource_id unfilled and raising TypeError.

Resolve the order URI via self.connection.get_resource_uri_from_id with Orders.resource_name, and update the test to assert payload dispatch.

Fixes WeMoveEU#16
@aaronelliotross
aaronelliotross self-requested a review October 7, 2026 09:29

@aaronelliotross aaronelliotross left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@DevMubarak1 Thanks for the PR, looks good ! From the language in the PR, I'm guessing this was an AI authored fix. Either way, thanks again !

@aaronelliotross
aaronelliotross merged commit e361b3a into WeMoveEU:main Oct 7, 2026
1 check passed
@DevMubarak1

Copy link
Copy Markdown
Contributor Author

@aaronelliotross Haha, busted on the description. Used an LLM to help draft the writeup but tested and verified the fix thoroughly. Appreciate the fast review and merge Aaron.

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.

Transactions.create(order_id=...) always raises TypeError

2 participants