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

Add delivery address to distribution "receipt" #4138

Closed
cielf opened this issue Feb 25, 2024 · 24 comments · Fixed by #4164 or #4540
Closed

Add delivery address to distribution "receipt" #4138

cielf opened this issue Feb 25, 2024 · 24 comments · Fixed by #4164 or #4540

Comments

@cielf
Copy link
Collaborator

cielf commented Feb 25, 2024

Summary

Add the address for the partner/program on the distribution printout, In addition change the prompt on program address to program / delivery address

Why?

For delivered items, having the address of the partner on this sheet will be a great help for the drop off volunteers

Details

  • Change the prompt for program address on the partner to "Program / delivery address (if different)"
  • If and only if the delivery type is delivery or shipping, add the address as "Delivery address" to the printed distribution
    -- this will be the program/delivery address if it is filled in, otherwise it will be the partner's address

Criteria for completion

  • [] information added to the printout as described.
  • [] tests to demonstrate that
@cielf cielf added Help Wanted Groomed + open to all! Difficulty—Beginner labels Feb 25, 2024
@kannans5
Copy link
Contributor

kannans5 commented Mar 2, 2024

Hi, are we talking about this report here?

Screenshot 2024-03-02 at 10 28 46 PM

If so, where would we want this address to be added?

@cielf
Copy link
Collaborator Author

cielf commented Mar 2, 2024

It is. I would add it under the "Issued to:" section. There's room there, and the flow works.

@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Mar 3, 2024
@cielf
Copy link
Collaborator Author

cielf commented Mar 3, 2024

@kannans5 I'm assuming that question means you want to work on this one, so I'm assigning you (so no one else takes it).

@kannans5
Copy link
Contributor

kannans5 commented Mar 5, 2024

yeah sure, I will work on it. One question

Change the prompt for program address on the partner to "Program / delivery address (if different)
  • where is this being currently displayed? Is it in the distribution printout?

@kannans5
Copy link
Contributor

kannans5 commented Mar 5, 2024

If the distribution delivery method is either shipped or delivered, the print will look like this, otherwise I am not changing anything. Does that sound ok?

Screenshot 2024-03-05 at 10 26 02 PM

@cielf
Copy link
Collaborator Author

cielf commented Mar 5, 2024

Yup, that looks about right.

@kannans5
Copy link
Contributor

kannans5 commented Mar 6, 2024

opened a PR, please review when you get a chance.

@cielf
Copy link
Collaborator Author

cielf commented Mar 16, 2024

Reopening this because in the pre-release testing on staging, a delivery distribution did not have an address on print out.

@cielf cielf reopened this Mar 16, 2024
Copy link
Contributor

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.

Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

@ShepherdXAutomation
Copy link

This would be my first issue. I want to try this one, but I'm very, very new. I've been programming for a long time. I've just never worked on a GitHub issue before. I read the README, and I think I understand how to get started as far as the branch and the pull request. What else would I need to do to try this? I think: I would clone the code down to my environment, then host it myself? Then try to fix the issue on my own version, apply testing, get a code review, then do a pull request? How can I pull up these reports though? Also, the README mentions using linux for development. Do you really need to use linux to work on this project? Or is that just a joke? Anyways, any help, or clue to the right direction would help. Should I take this to the slack channel? How do I get there?

@dorner
Copy link
Collaborator

dorner commented May 2, 2024

Thanks for your interest! Here's the overall approach.

  • Fork the repo.
  • Clone your repo to your local machine.
  • Follow the application setup instructions to get it running.
  • Create a new branch locally.
  • Make your changes, and push them to your fork.
  • Create a pull request in this repo.
  • The pull request gets reviewed, tests and other CI steps are run.
  • If there are any failures or requested changes, make the changes and push them to your fork again.
  • Repeat the last two steps until all is good.
  • The branch gets merged and work is complete!

You do not need to work on Linux! Plenty of folks have it working on Mac or Windows.

If you have any further questions, you can get to the Slack channel here: https://join.slack.com/t/rubyforgood/shared_invite/zt-21pyz2ab8-H6JgQfGGI0Ab6MfNOZRIQA

@ShepherdXAutomation
Copy link

Thank you! I feel like I can accomplish this task. The previous user showed a picture of a document containing delivery information. How do I get this data for testing? Does it come with the repo as example data?

@dorner
Copy link
Collaborator

dorner commented May 2, 2024

@ShepherdXAutomation yes, when you set up your database you'll get a bunch of seed data for free. :) You can see the document by:

  • Log in as [email protected] with password password!
  • Click "Distributions"
  • Find one and click "Print"

@cielf
Copy link
Collaborator Author

cielf commented May 2, 2024

@ShepherdXAutomation Fair warning - it might just be that there is some weird case that I saw on staging to yank it back. I've also seen it work there

@ShepherdXAutomation
Copy link

Thank you all so much for the guidance. I’ll see how it goes. Even if I don’t solve it, getting the app setup and the seed data in place, will be good thing.

@cielf
Copy link
Collaborator Author

cielf commented May 4, 2024

Definitely!

@jimmyli97
Copy link
Contributor

I'm happy to take this one, I think it's just a typo. Also, I can try to write tests using the pdf-inspector gem

@cielf
Copy link
Collaborator Author

cielf commented Jul 3, 2024

It's yours. I think it would be cool to test the pdf if that gem is solid -- @dorner - do you have knowledge/ an opinion on it?

@jimmyli97
Copy link
Contributor

The gem was written to test the prawn gem which is what the human-essentials app uses to generate pdfs

@dorner
Copy link
Collaborator

dorner commented Jul 4, 2024

Might be overkill for this PR. The approach I tend to use when trying to test generated documents is to freeze time or any other randomized element that might be used, generate the document, save it to the test directory, and update the test to compare the generated file against the one in the repo.

Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

jimmyli97 added a commit to jimmyli97/human-essentials that referenced this issue Jul 18, 2024
…ethod

* Address output prints delivery address if filled in, otherwise partner address
* Only does this for delivery/shipped method
jimmyli97 added a commit to jimmyli97/human-essentials that referenced this issue Jul 19, 2024
…ethod

* Address output prints delivery address if filled in, otherwise partner address
* Only does this for delivery/shipped method
@github-actions github-actions bot removed the stale label Jul 26, 2024
Copy link
Contributor

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.

Copy link
Contributor

github-actions bot commented Sep 1, 2024

Automatically unassigned after 7 days of inactivity.

dorner pushed a commit that referenced this issue Jan 16, 2025
* REFACTOR group examples, remove rubocop disable lines,

* group examples together with variables
* rubocop -A passes on this file without needing to disable ArrayAlignment

* RED add rspec to test address output in distribution PDFs

* Compare generated against expected PDFs
* Add helper test to regenerate expected PDFs
* Add helper module

* GREEN Fix #4138 address output changes based on delivery method

* Address output prints delivery address if filled in, otherwise partner address
* Only does this for delivery/shipped method

* BUGFIX pdf now writes when test fails

* RED add rspec for when partner has no addresses

* REFACTOR out profile name and email

* GREEN don't print address if no address or delivery address

* Add comments clarifying gitignore and comparison pdf helper rspec

* Update bundler version

* REFACTOR replace helper test with environment variable

* Use environment variable for regenerating comparison pdfs

* update schema date and bundler version

* RED remove env variable code, stub out console helper method

* GREEN add Rails console method for generating comparison pdfs, update comparison PDFs

* Replace instance vars with structs and let

* FIX destroy request so db is clean, use destroy! instead so exceptions fire

* Replace FactoryBot with calling ActiveModel.create, move methods to lib/

* Fix formatting when address is incomplete, add rspecs, create and delete item units correctly when generating test files

* FIX replace with space if partner primary contact name/email/phone is blank

* Add rspec
* Other comparison pdfs change because previously blank phone # skipped line,
  now blank phone # adds a blank line
* Move instance_method call inside function

* FIX spacing between issued to and delivery address

* RED Make methods module instead of instance

* Move helper compare_pdf method into Rspec group
* Failing tests are due to Items received year to date being inaccurate in new year

* GREEN fix test and comparison pdf generation errors

* travel_to time when creating distribution so Items received YTD is always the same
* use public_send
* regenerate comparison PDFs, contents are identical, only binaries differ (possibly because of prawn-rails update to 1.6.0?)
* write expected_file_path to filename when outputting non matching PDF

* Fix linting, use block for travel_to

* Switch to using transactions with rollback to clean up

* Remove accidental duped tests on merge, move merged test into example group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants