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

Customer credit ledger entry creation, and customer credit enumeration #18

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

arusahni
Copy link
Contributor

@arusahni arusahni commented Nov 27, 2023

This adds support for fetching the credit balances associated with the customer. To support testing this functionality, this also adds support for creating a subset of customer ledger entry types (credit, void).

I'm very much open to some bikeshedding on the naming for the ledger entry base type. Staring at this for too long had me feeling smurfy.

It also bumps the MSRV because some deps use once_cell and we want to use the stabilized version.

@arusahni arusahni requested a review from benesch November 27, 2023 22:56
@arusahni arusahni force-pushed the feat/customer-credit-ops branch from ccf5fd7 to d33ed09 Compare November 27, 2023 22:57
@benesch
Copy link
Member

benesch commented Nov 28, 2023

@arusahni would you mind conscripting someone else for this review? I took a quick skim but I won't have time to go deeper—and it's long past time for me to get out of the business of maintainership of this crate entirely!

@arusahni arusahni force-pushed the feat/customer-credit-ops branch from 484dade to 99ce79e Compare November 28, 2023 19:08
Copy link

@guswynn guswynn left a comment

Choose a reason for hiding this comment

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

I didn't verify that the structs map to the document orb structs, but the serde stuff looks good to me, minus a few nits!

nice use of lifetimes!

src/client/customers.rs Show resolved Hide resolved
src/client/customers.rs Show resolved Hide resolved
src/client/customers.rs Show resolved Hide resolved
src/client/customers.rs Show resolved Hide resolved
src/client/customers.rs Show resolved Hide resolved
@edude03
Copy link

edude03 commented Nov 28, 2023

Looks like it'll work 🆗

@arusahni
Copy link
Contributor Author

@benesch could I get another admin merge here since I had to bump the MSRV again?

@benesch
Copy link
Member

benesch commented Nov 29, 2023

@benesch could I get another admin merge here since I had to bump the MSRV again?

I bumped the MSRV in the required checks, too. Will merge, to lock it in, but you should also be empowered to merge yourself too, going forward!

@benesch benesch merged commit 28cd924 into main Nov 29, 2023
6 checks passed
@benesch benesch deleted the feat/customer-credit-ops branch November 29, 2023 04:44
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.

4 participants