Skip to content

Fix missing description on customer credit transactions created via the API#14432

Merged
dacook merged 1 commit into
openfoodfoundation:masterfrom
emilythericky:14407-fix-customer-credit-description
Jul 9, 2026
Merged

Fix missing description on customer credit transactions created via the API#14432
dacook merged 1 commit into
openfoodfoundation:masterfrom
emilythericky:14407-fix-customer-credit-description

Conversation

@emilythericky

Copy link
Copy Markdown
Contributor

What? Why?

When a client creates a customer credit via POST /api/v1/customer_account_transaction with a properly wrapped request body — { "customer_account_transaction": { "customer_id": 1, "amount": "10.25", "description": "Payment processed by POS" } } — the transaction's description came back empty ("API credit: " with nothing after it). The note explaining what the payment was for was silently dropped.

Root cause: the controller reads every field from inside the wrapped customer_account_transaction params except the description, which it mistakenly read from the top level of the request (params[:description]). For a correctly wrapped body there is no top-level description, so it came through as nil.

The existing request specs didn't catch this because they send the body unwrapped (flat keys). Rails' wrap_parameters copies those flat keys to the top level too, so params[:description] happened to work in the tests but not for a real wrapped request.

This is a pre-existing bug, found while testing the API v1 toggle removal (#14012).

Changes

  • app/controllers/api/v1/customer_account_transaction_controller.rb — read the description from customer_account_transaction_params[:description], consistent with how every other field is read.
  • spec/requests/api/v1/customer_account_transaction_spec.rb — added a regression test that posts a properly wrapped body and asserts the description is preserved. It fails on the old code (got: "API credit: ") and passes with
    the fix.

What should we test?

  • Send POST /api/v1/customer_account_transaction with a wrapped JSON body containing a description, e.g. { "customer_account_transaction": { "customer_id": <id>, "amount": "10.25", "description": "Payment processed by POS" } }.
  • Confirm the created transaction's description is "API credit: Payment processed by POS" (previously it was just "API credit: ").

Release notes

Changelog Category:

  • API changes (V0, V1, DFC or Webhook)

@sigmundpetersen

Copy link
Copy Markdown
Contributor

Hi @emilythericky 👋
Was this created with the help of AI?
We have created an ai label to keep track of the amount of AI assisted PRs.
I can label it for you if it's the case 🙂

@emilythericky

emilythericky commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Hi @sigmundpetersen, yes I got AI assistance with this bug. Thanks!

Also, the failed test is unrelated -- could someone rerun it or ignore it?

@sigmundpetersen sigmundpetersen added the ai AI generated or assisted PR/Issue label Jun 18, 2026
@sigmundpetersen

Copy link
Copy Markdown
Contributor

Thanks, everything green after a rerun.

Just noting also, that we prefer to have 1 pr/issue per contributor simultaneously in the pipe (OFN Delivery board).
This is to balance the load on the code reviewers and testers.
I see you have a couple already on the board so let's wait until those are through before moving this one forward 👍

I guess this could be a feesback point to the AI CoP

@emilythericky

Copy link
Copy Markdown
Contributor Author

Hi @sigmundpetersen thanks for letting me know, I wasn't aware of the 1 pr/contributor rule. This bug was assigned to me by @rioug after I found it while working on #14012, which was past due for the UK digital inclusion project, also assigned to me by @rioug -- I would defer to him (and @mkllnk) as to whether to deprioritize this, but it does seem relevant to a funded project and it's only 2 lines of code + test file...

Hey, I'm wondering if you guys could use the new AI tag to help direct our AI CoP cohort toward issues that are highest priority and suitable for us to work on?

@mkllnk

mkllnk commented Jun 19, 2026

Copy link
Copy Markdown
Member

we prefer to have 1 pr/issue per contributor simultaneously in the pipe (OFN Delivery board).

I didn't know this either. 🤷

I'm sorry if you feel like you did something wrong. You didn't. We are currently behind in our roadmap and found that the core dev team is a bottleneck for code review and roadmap tasks. So we are discussing how we resolve this.

Your contributions are all useful and I'm glad that things move forward at a good pace. I'm just wondering how we can further reduce the load on the core team. So we may not be as responsive to code review over the next weeks as some deadlines are coming up.

@sigmundpetersen

Copy link
Copy Markdown
Contributor

we prefer to have 1 pr/issue per contributor simultaneously in the pipe (OFN Delivery board).

I didn't know this either. 🤷

Yes I don't know if it's consensus, it was discussed here https://openfoodnetwork.slack.com/archives/C01T75H6G0Z/p1781678655460899

Sure if this is part of funded work we should move it forward, no problem.

I guess all this is part of the experiment and we should try to find positive outcomes and how AI may help us.

@sigmundpetersen sigmundpetersen moved this from All the things 💤 to Code review 🔎 in OFN Delivery board Jun 19, 2026
@rioug rioug added the api changes These pull requests change the API and can break integrations label Jul 3, 2026

@rioug rioug 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.

Looks good, thanks @emilythericky 🙏

@chahmedejaz chahmedejaz 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.

Looks good 👍

@chahmedejaz chahmedejaz moved this from Code review 🔎 to Test Ready 🧪 in OFN Delivery board Jul 5, 2026
@mariocarabotta mariocarabotta added the priority We focus on this issue right now label Jul 8, 2026
@AMEA-LYON

Copy link
Copy Markdown

Perfect ! Good job ! 🎉

14432_API

@AMEA-LYON AMEA-LYON self-assigned this Jul 8, 2026
@AMEA-LYON AMEA-LYON moved this from Test Ready 🧪 to Ready to go 🚀 in OFN Delivery board Jul 8, 2026
@dacook dacook merged commit 64a3eaa into openfoodfoundation:master Jul 9, 2026
68 of 69 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to go 🚀 to Done in OFN Delivery board Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai AI generated or assisted PR/Issue api changes These pull requests change the API and can break integrations priority We focus on this issue right now

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

API v1 customer_account_transaction silently drops description for wrapped request bodies

8 participants