Add the distribution ID in the "receipt" (i.e. distribution printout)

[cielf]

Summary

Add the distribution ID to the distribution printout

Why

It serves the same purpose as an invoice number -- will help the banks with their off-line record keeping of pickups.

Details

##To see a distribution printout,
sign in as [email protected] Click Distributions Then click "print" beside a distribution. This displays a pdf ##Change required: Insert the field just above "Comments", on the left side of that pdf, with the heading "Distribution ID", (and show the distribution id there).

Criteria for completion

• [ ] behaviour as described
• [ ] automated tests to support the change
• [ ] update user guide (relevant page at docs/user_guide/bank/essentials_distribution.md)

[sunsheeppoplar]

@cielf 👋 I've been working a solution this weekend. I think I'll be able to put up the fix soon, just working through some issues generating the test pdfs correctly. Any chance I could be assigned this?

[cielf]

Yup! Done. Good to see you back.

[sunsheeppoplar]

@cielf thanks, glad to be contributing again. I've run into a bit of a snag with tests. I'm not sure what the maintainers would like to see here.

I see that not too long ago there were some tests added to do some pdf comparisons which is pretty cool. By adding the requirement of rendering the distribution id, as it stands, it would mean that every variation would need to be accounted for in fixtures/files/*.pdf because every call in the spec of create_dist is incrementing an id causing failures in the compare_pdf comparison.

Right now there are 11 calls of the create_dist helper. I'm not sure if it's acceptable to commit all 11 fixtures or if there's something else more creative we could do. Over time I worry this wouldn't scale that well. Let me know what you think!

[cielf]

I'm going to pass that question over to @dorner.

[dorner]

Unfortunately you can't use the ID directly in the PDF, because the ID will be different depending on the order of the specs passing.

Here's my recommendation: Add the value {{distribution_id}} in the fixture PDFs where the actual distribution ID would go. Then update the comparison function to do a find/replace on {{distribution_id}} and replace with the actual distribution ID within the test. Then when you compare them it should pass.

[sunsheeppoplar]

@dorner Thanks for your help. I think I understand the gist of what you're saying, but I'm struggling a little with the mechanics.

1. Temporarily add {{distrubtion_id}} to the Prawn rendering code in distribution_pdf.rb
2. Regenerate the fixtures
3. In compare_pdf use IO.binread on the expected file

Assuming all that is correct, I wasn't sure how to go about finding and replacing. Would I need to convert the the string {{distrubtion_id}} into the format the binread and binwrite use, ASCII-8BIT?

[dorner]

hmm... maybe? I'd try without it and if it doesn't work, you can convert the string. I'm not 100% sure either of those will work though...

You might need to hardcode the distribution ID when you generate them (e.g. set distribution.id = 456 before passing into the generator function)

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[github-actions[bot]]

Automatically unassigned after 7 days of inactivity.

[cielf]

Available for someone else to pick up.

[sunsheeppoplar]

Sorry @cielf I meant to write back. I can take another look into what dorner suggested but based on what I saw initially there wasn't an easy way to even hardcode them. I can report back again if you want to reassign me

[cielf]

@sunsheeppoplar Do you have a solution, just not with the tests working? If so, maybe put it up as a draft and ask for someone to look at what you have?

[wjwb-us]

@cielf I'll pick this one up, got a solution with working tests just need to update the user guides 😊

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[cielf]

@wjwb-uk -- checking in to see if we'll get your solution soon.

[github-actions[bot]]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[github-actions[bot]]

Automatically unassigned after 7 days of inactivity.

[cielf]

Without a pull request, I'm putting this one back out there for whoever would like to take it on.

[kellis5137]

Hi, I would like to pick this up... seems simple to fix... but question... The respec test code is using PDF test fixtures to compare. Are you all using any particular PDF editor?

[cielf]

Passing that question to @dorner . Also assigning to @kellis5137

[kellis5137]

@cielf @dorner - I figured it out... I think I've got everything I need.

[kellis5137]

Finished it (I believe). I thought I squashed the commits before sending, but apparently I did something wrong. In any case, here is the PR https://github.com/rubyforgood/human-essentials/pull/5278