4218 customize distribution receipt #4234
Conversation
|
Just re read the issue, I believe I may have misread it. |
|
Sorry this took so long, work / life balance... |
nikolaz90
changed the title
|
You can try passing an explicit name to your item to avoid this. |
…laz90/human-essentials into 4218_customize_distribution_receipt
|
Hey @nikolaz90 -- If I run bin/setup, then rails db:migrate I'm getting duplicate column errors. Might be a problem on my end, but thought you should know. |
|
Wow, my bad, I committed the wrong timestamp on the schema.rb on resolving the conflict... Thanks for letting me know and please accept my apologies. I subsequently ran bin/setup and that is now functionning. |
| @@ -11,6 +11,8 @@ | |||
| # enable_child_based_requests :boolean default(TRUE), not null | |||
| # enable_individual_requests :boolean default(TRUE), not null | |||
| # enable_quantity_based_requests :boolean default(TRUE), not null | |||
| # hide_package_column_on_receipt :boolean default(FALSE) | |||
| # hide_value_columns_on_receipt :boolean default(FALSE) | |||
There was a problem hiding this comment.
@cielf I'm kind of starting to feel like some of these fields belong in a "settings" model. E.g. these and ytd_on_distribution_printout - these are suuuper particular to be sitting on essentially our "god model". Thoughts?
There was a problem hiding this comment.
There is a convenience factor in having the organization view looking just at the organization model, I suppose. I do agree that a lot of these are really really specific.
There was a problem hiding this comment.
Let's talk about it on Sunday -- it would be a new issue, in any case.
There was a problem hiding this comment.
Happy to refactor this PR to include a settings table, or print options table if that helps
app/pdfs/distribution_pdf.rb
Outdated
| col_index = data.first.find_index(col_name) | ||
| data.each { |line| line.delete_at(col_index) } if col_index.present? | ||
| end | ||
| data |
There was a problem hiding this comment.
There isn't really a reason to return data here since the operation is mutating the object itself. You can just call it like a method.
There was a problem hiding this comment.
Thanks, just updated that.
|
This looks good to me, but there are conflicts with main @nikolaz90 - can you fix? @cielf did you want to take a look? |
|
I should be able to kick the tires tomorrow morning. I already did once, so I expect it will be quick. |
|
Hello! |
|
Working just fine on my functional check. |
|
As for the test, I just ran that test on main on my local, and it failed too. If I'm reading things right, we just introduced a change in that test last night. @dorner -- how would you like to proceed -- do you want to roll back that change? |
|
Opened a PR for this #4347 |
|
All good - thanks! |
|
@nikolaz90: Your PR |
Resolves #4218
Description
This adds the ability to hide Value and/or package related columns on a printed receipt
Type of change
How Has This Been Tested?
Specs added to DistributionPDF spec.
Specs ran and passed
Log in as organization admin and go to /diaper_bank/manage/edit, 2 radios buttons to hide value and/or package columns on distribution receipts.
Edit :
Show:
Pdf with different columns hidden :