Skip to content

Simplify Receipts endpoint#359

Open
marfavi wants to merge 7 commits into
mainfrom
rewrite-receipts
Open

Simplify Receipts endpoint#359
marfavi wants to merge 7 commits into
mainfrom
rewrite-receipts

Conversation

@marfavi

@marfavi marfavi commented May 3, 2026

Copy link
Copy Markdown
Member

Reworked GET /api/v2/receipts to return ReceiptsResponse with a flat ReceiptListItem list. It doesn't have subclasses but we make guarantees about whether some values are null or not based on the Type discriminator.

Why: Our OpenAPI spec generator (NSwag) can not properly describe polymorphic classes. This made it hard for the app to consume this endpoint.


Removed pagination - all receipts are returned in one call.

Why: Reduces complexity on the client and server. The median amount of receipts per user is very small at ~50, and the 90th percentile is ~210, so we don't expect this to cause performance issues. The app also only renders the receipts visible on screen, so there's no need for pagination for rendering performance.


Removed the ability to filter by receipt type in the request.

Why: Natural consequence of the previous two changes. The client can easily filter the results on its end, and this keeps the API simpler.


Excluded Free purchases from receipt retrieval so they do not appear as purchase receipts.

Why: Free "purchases" are not monetary purchases in the receipt sense. They are effectively the source records for consumed perks, and they already surface through the UsedTicket receipt path. If the service included them in the purchase receipt bucket, the user would see misleading duplicate activity (once as a “purchase”, and again as a used ticket).


Renamed ReceiptController -> ReceiptsController.

Why: To match the other controllers.

@marfavi marfavi requested review from TTA777 and duckth May 3, 2026 18:51
@marfavi marfavi mentioned this pull request May 3, 2026
@TTA777

TTA777 commented May 3, 2026

Copy link
Copy Markdown
Member

To address your points, and why I currently have no intention of approving this PR.

Re: The open api spec. As far as I can see NSwag is generating it exactly as it should be, when I look at the spec itself. It did after all also manage to generate a correct consuming client for our integration test suite. What in the generated spec is wrong? From what I can see, all inheriting classes are currently referencing the base using "allOf" in the spec, with correct "mapping" field on the discriminator in the base.

Re: pagination. My concern is not for the 50th, or 90th percentile, but the 99th. There are users in the system with 1000+ receipts, and it keeps growing after all. Would it cause performance issue, to return all? Probably not, but we might as well use a little bit of foresight to make it built to last. Implementing pagination now is easy, adding it later, not so much.

Re: Free purchases, we could filter those out, considering the user is not aware there is a "purchase" and only really sees the swipe taking place. I don't mind that change.

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.

2 participants