kasugaijin / baja-pet-rescue

Kasugaijin's open-source Ruby on Rails production app that enables dog rescue organisation staff in Mexico to post dogs and receive applications for adoption from users in USA and Canada..

• 👋 Hi, I’m Ben, @kasugaijin
• 👀 I’m interested in ... hiking with my dog and heading in the bush in my Land Rover Disco I.
• 📫 How to reach me ... www.linkedin.com/in/robinsonb
• Fun Fact: I lived in Kasugai, Japan for a year, and my username is a play on being a (foreign) resident in the city.

baja-pet-rescue's Issues

Use fixture references instead of active record queries in test setups and assertions

In multiple cases throughout the test files the setup variables and some assertions are using active record queries e.g., searching for a record by a name. This is a brittle approach, because if the fixture property changes, a query on this property may return nil and break the test. Therefore, the tests should make use of fixture references (e.g., dogs(:dog_two) instead of Dog.find_by(name: "rover") ) as much as makes sense. This allows fixture attributes to change without unintended side effects.

Acceptance criteria:

• where it makes sense, change active record queries to fixture references
• all tests should pass
• changing a fixture attribute should not break the tests

Create tiles of testimonials on the successes page and rename page to happy tails

As a user visiting /happy-tails, under the map I should see six tiles, each with a picture of a dog with its new family and a short testomonial.

Change route from /successes to /happy-tails

Determine why adopter profile form radio button styling breaks in Firefox

Broken styling has been reported when filling in the adopter profile form using Firefox. The labels should be on the same line as the radio button (yes, no). This ticket is to try and recreate the issue and dermine what might be the cause. Or if you cannot recreate, let me know and we can close this issue.

Navbar - switch positions of Donate and Successes

When I look at the Navbar, the Donate link should be where Successes link is and vice versa.

Test adopted dogs can't be shown as an adoptable dog

adoptable_dogs_controller.rb show action will take a user to /adoptable_dogs/:id unless the dog is adopted. In that case the user is redirected with a flash.

Acceptance criteria:

• test shows that trying to access show action in adoptable_dogs controller using an id of an adopted dog causes a redirect with flash.
• all tests pass.
• add the test to adoptable_dog_show_test.rb

Purge attachments in test environment after each test run

Rails does not automatically purge attachments that are created during testing e.g., in this app, in org_dogs_test,rb

See the rails guides on testing active storage.

Acceptance Criteria:

• Create method that clears attachments at the end of each run of the test suite

Adopter profile reviews test

Acceptance criteria:

• create a new file: adopter_profile_review.rb
• add a test to show a verified staff can access an adopter profile page (response success and assert the h1 text)
• add a test to show unverified staff get redirected with flash error if they try to access an adopter page
• all tests pass

Add tests for successes page

The /successes page uses Google Map API integration to show pins of the cities and towns where dogs have been relocated to. This uses a controller to create a collection of locations that get sent to the view, and Stimulus JS to create a list of elements with attributes for lat and lon.

The existence of elements and their attributes should be tested. Given the output is created by javascript client side, it will probably have to be a system test, rather than integration.

Test an email is sent when a user goes through the Forgot Password flow

We are missing a test in user_account_test.rb for the forgot password flow. This issue is to create that test, making sure the user receives an email. See the examples of using ActionMailer in this file if necessary.

Acceptance Criteria:

• test checking email is sent when user goes through Forgot Password flow
• all tests pass

Test adopter applications page

/my_applications
This route shows the applications a given adopter user has made. It should show the dog, the status and a button to withdraw and remove. This functionality should be tested to ensure it works. See the test fixtures for setup - there should be one adoption application for user_one, for dog called 'deleted'.

Acceptance Criteria

• create a new file adopter_applications_page_test.rb and put in test/integration folder
• test presence of expected elements, e.g., Dog name, status, button
• check the buttons work e.g., submit request when clicking withdraw, and then expect the button to say 'remove', then do the same for remove and expect the application to disappear.
• tests pass

Review Code with Jason webinar (for testing) and make a list of refactor suggestions

https://www.youtube.com/watch?v=AD5Sg-CKgww

Similar to #121. I think Jason provided very valuable feedback and some of those changes could definitely be incorporated into this project.

I am very interested, so I will keep a lookout on this issue over the next 7 days. If no one takes it, I will request to be assigned to it.

I would love to give someone a chance to work on this before me. :}

Test revoke adoption feature

Acceptance criteria:

• test that revoke adoption button appears on org dogs #show
• test that revoke adoption deletes the adoption and sets the successful application status to withdrawn
• test an adopted dog does not appear on the adoptable dogs #index, and does appear after revoking the adoption

Tests for Contact Form

/contacts/new
Create a new integration test file contact_form_test.rb in test/integration and check that the form errors are displaying as expected. Note that the UI also makes use of required in the HTML, so you will see the default browser errors if you try to replicate the errors in the UI. Therefore, you can temporarily remove required in that form's view file and then submit the form to see the Rails error messages based on the model validation.

Acceptance Criteria

• a new file contact_form_test.rb in the correct folder test/integration
• tests checking presence of errors for missing name, email and message fields

Modify duplicate locations used for google map API

The lat long locations used for the google map API on /successes are generated using city/town, province/state and country of the adopter. Therefore, if two or more dogs end up going to the same city or town they have identical coordinates. This means that multiple pins overlap on the map, hiding them from view.

This ticket is to check the coordinates for duplicates and modify the duplicates with a minor coordinate change so the pins show up in different places. Given the pins do not need to be precise, and we are shielding actual locations from view, this approach is sufficient.

Test user gets redirected to root page on sign in/sign out

`user_account_test.rb'

Acceptance criteria:

• a test to show a user gets redirected to the root path on sign in, and again on sign out
• tests pass

Tests for the custom errors controller

The application has custom error responses for when a 404, 500 or 422 hits the routes file (see match verbs). These are sent to the errors_controller.rb to render the appropriate view. These are not tested.

This issue is to re-create a request that will cause each of the three errors to be produced and assert that the expected content is displayed i.e., the error code and message.

Acceptance criteria:

• test for each of the three error codes asserting expected view file is rendered
• all tests pass

Update successes page with client copy

Once copy is received from client, update the text below the Google Map.

System test for adopter profile new/edit form

The adopter profile _form partial uses stimulus (JavaScript) to dynamically manipulate the DOM in some instances e.g., insert and increment/decrement a character counter when a user is filling in a text area, or show/hide an additional form field if a certain radio button is true/false.

It would be beneficial to test this JavaScript functionality is working as expected when a user is interacting with a form. A system test where we can assert DOM element presence would be required.

Acceptance Criteria:
System test that mimics a user interacting with the form and checks:

• conditional fields show/hide when a given radio is true/false
• character counter is displayed and increments/decrements with text being entered/removed from a text area
• checks for validation prompts if a required field is empty on form submit
• all tests pass

Update:
#68 meets above criteria except check that text area disappears with click
#69 adds checks that text areas disappear with radio click

Remove applications with one click after an adoption is made

When an adoption is made, the status of 'adoption made' for that dog shows on each adopter user's applications page. The user then needs to make two clicks to withdraw and remove this application to get rid of it. The same applies for the successful adopter.

This can be improved. It would be better, if after an adoption is made, there is a single click to remove the application from the applications page.

This ticket is to investigate the best way to implement this logic.

Spike - determine the best way to test the mailers

This issue is to investigate the best way to test the sign up mailers are working properly. Emails are sent:

• to a new adopter when they sign up
• to a new staff when they sign up
• to admin when a new staff signs up
• to staff when a new adoption application is made

I am not sure the best way to conduct these tests e.g., all in one mailer test file, or separate files. This ticket is a placeholder for this research and discussion.
.
.
.
Unit Tests
Unit tests will be useful to test the mailer class and ensure it returns the expected parameters e.g., to email, from email, email subject, email body and any other parameters we need to confirm. There is some overlap with the integration test below, but it would be useful to test the body contains the expected content.

Integration Tests
Integration tests will be useful to test emails are being sent during specific user flows (see above list). These are easily tested for by ensuring config/test.rb is set to config.action_mailer.delivery_method = :test, which tells Action Mailer not to deliver emails to the real world and instead accumulate sent emails in the ActionMailer::Base.deliveries array. Emails in this array can be tested for presence and the params of date, from email, to email and subject line. This array just needs to be cleared each time so you can reliably call the first element in each subsequent test and get the email from that test.

Test the adoption application review page filter works

adoption_application_reviews_controller.rb has a private method selected_dog. This is used by staff on the applications review page to allow them to filter results to one dog. This should be tested to ensure making the request with a selected dog returns that dog only and its application(s).

Acceptance criteria

• test verifies selecting a dog returns that dog
• add test to adoption_application_reviews_test.rb
• all tests pass

Integration tests for mailers

Mailers for new sign ups and adoption applications should be tested to ensure email is sent and with the correct parameters (e.g., to email, from email and subject)

Acceptance Criteria

• test for new adopter user welcome email
• test for new staff user welcome email
• test for new adoption application email to staff
• test for new staff user email to admin
• all tests pass

Streamline local setup process in README

First off, this is a lovely project 🐶

When cloning this I noticed a couple of steps in the local environment setup could be streamlined some.

• When a user does ruby -v, this is going to show the ruby version they currently have globally set. This project's .ruby-version file will automatically tell rbenv (or rvm) to use the required version of Ruby when the user is in this project. This means that devs don't necessarily have to have their global version set to 3.1.1, they just have to have that version installed and available for their version management tool to use. With rbenv, this means running rbenv versions and checking that 3.1.1 is in the list, and with rvm, running rvm list rubies and checking for the same.
• These three commands here can all be run together with just a rails db:setup

Would love to make a PR if that all seems fine with you.

Test for organization dogs

/dogs
When signed in as staff and accessing this route, you will see a list of all dogs under that organization, and here can filter the page by adoption status and name, and create/edit/update/delete a dog.

Acceptance Criteria

• get the test working for uploading an image to active storage, it is currently commented out
• add a test to check the pause applications form for a given dog is working, and the appropriate pause reason is being saved to the Dog
• add tests to check the filters work as expected for adoption status and name**
• all tests pass

Possible dead code in app/services?

While working on service object #160 I noticed that there might be a possible dead code in app/services/geocode_backfilled_location.rb the class GeocodeBackfilledLocation itself is not used anywhere, I wonder are you keeping this code for future use or it's forgotten?

Test contact mailer is appropriately sending emails

In the file created in #44 add a test to ensure the submitted message is sending the appropriate email.

Acceptance Criteria

• Test checks email is sent after submission of form (providing no form errors)
• Tests pass

Force SSL in config

force application to use ssl
https://help.heroku.com/J2R1S4T8/can-heroku-force-an-application-to-use-ssl-tls

Add donation to active admin

I should be able to log into active admin and see the donation total for each currency shown on the landing page in the existing list of basic stats

Run traceroute to look for any dead routes or unused controller actions

Per this article best practice is to ensure controller methods are exposed only as needed. There should be no public controller method that does not support a route.

This ticket is to perform the exercise of looking for dead routes and/or inappropriately exposed controller methods. This can be done by hand or using a gem like traceroute.

Use newer version of bundler?

When running rake traceroute, rails test or rails test:all, I always get this warning:

Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.

I remember looking into a fix for this back in January when I first started working on this app, and the solution is to basically use a newer version of bundler than the one that this app is using. I remember running bundle update --bundler on my cloned repo, and that fixed it.

When I wanted to push my updated Gemfile.lock today, I realized that my cloned repo's Gemfile.lock is using newer versions of many gems than this original repo's Gemfile.lock. So instead of pushing that file with all the newer versions of the gems, I wanted to keep it simple, and keep things in line with this repo's gem versions. I copy pasted this original repo's Gemfile.lock contents into my Gemfile.lock and ran bundle install to add the Traceroute gem. Then I ran rails test:all, and noticed that the old spell_checker warning was back again, because the bundler version got reverted back to the older one that this original repo is using.

That's a long-winded explanation, but I thought the context might be helpful. Basically, if you also get this spell_checker error/warning, we could fix it pretty easily by using a newer version of bundler by running bundle update --bundler. Would love to know what you think!

Prepare website for summer down season

The organization's efforts are largely seasonal, with the adoption period typically extending from Oct to June. So, I need to confirm with the client if they intend to cease adoptions or at least will ramp things down and likely have fewer dogs and longer response times. The website should be updated accordingly so that new visitors are aware.

Possible places:

• Hero - add sentence stating off season dates
• About - add a paragraph to about us page
• Other ?

Tests for staff reviews of adopter applications

/adopter_applications route

Staff can review applications at this route. We should create a new integration test file to test the features on this page, including tests for:

• staff see all adopter applications
• staff can edit an adoption application status
• staff can add notes to an application
• when 'successful' applicant is selected, button to 'create adoption' shows
• after making the http request to create an adoption the application disappears and a new Adoption is created
• an adopter can withdraw and remove an application and a staff can subsequently set the adoption application from withdrawn to another status and the application reappears on the adopter's applications page

Add url to production site to Readme

I cannot believe this is not in there already!
https://www.bajapetrescue.com

Make footer copywrite year dynamic

The footer copywrite year is static and set to 2022. This should be dynamic so it updates the year automatically.

A couple possible improvements for '/adopter_profile/new'

For '/adopter_profile/new':

• It might be better to rename 'Week day' to 'Weekday'
• I use Firefox in my VM while working on this app, and I noticed that all of the 'Yes/No' radio buttons are not aligned properly. @kasugaijin I can send you a screenshot in Discord to illustrate what I mean. I tried it out in Chrome, and there are no alignment issues there

Refactor coordinates class to apply random to all coordinates

Instead of applying random deviations to duplicate coordinates only, just apply random to all. This will allow you to easily pass more data attributes.

Communicate adoption pending status on unadopted dogs

If an application status is set to adoptiong_pending on a dog, this should be communicated on the adoptable dogs index and that dog's show page. This will inform potential adopters this dog may no longer soon be available.

Acceptance Criteria

• User sees notification that a given unadopted dog has status adoption pending
• This updates automatically with staff setting this status on a given application
• Appears after dog name on index and show
• integration tests capture expected behaviour

Exchange explict count assertions with more flexible active record collection counts

The test suite is starting to exhibit brittleness due to assertions that expect an explicit count e.g., 3 of a given element. This means if you add another fixture, these tests break because the count has increase by one. Such assertions should be replaced with active record queries that count a collection. Therefore if you add one more record in the fixtures, it will be included in this count and the test won't break for this reason.

This ticket is to focus on the explicit count assertions in test/integration/adopter_applications_page_test.rb

Acceptance criteria:

• explicit counts exchanged with active record queries that count a collection
• tests pass if a new record is added in the fixtures
• all tests pass

Add ability to undo an adoption

In rare cases an adoption falls through. This means the adoption should be deleted so the dog returns back to the adoptable dogs index.
The status on the successful adopter application should be set to withdrawn, and the statuses for all other applications set to awaiting review (from adoption made).

Acceptance criteria

• - staff can find the adopted dog in their org dogs list by filtering for adopted and on dog > show, a button to delete adoption should allow them to delete it. A flash warning before submitting the request should show.

• - the adopter application of the successful applicant - the status is set from adoption made to wtihdrawn

• - the status of all other applications is set from adoption made to awaiting review This step is no longer required. Staff can manually choose the appropriate status for other applications after reverting an adoption. There is no single status that makes sense to revert all applications to.

Intermittent Test Issue in Adopter Profile Form Integration Test

After successfully cloning & initializing the db/seeds,
I was getting errors on the default test suite (without having made any modifications whatsoever).

The error:

undefined method `adopter_account' for nil:NilClass (NoMethodError) 

it pointed to line 5 within test/integration/adopter_profile_form_test.rb
which contains:

adopter_account_id = User.find_by(email: '[email protected]').adopter_account.id

I even checked the database (in the console), and I could still see the User in your fixtures.
However, after running the tests a few more times, the issue was gone.

Proposed solution: Use the setup method with an instance variable that references the stored fixture. I believe this would be more reliable than using the local variable within the test class. Please let me know if you have other ideas!

Related PR: #52

Add payment form for donations

The site would benefit from a simple donation form on the /donate route. Dicussion with the client concluded that Paypal is the most suitable payment processor to use as it is the one currently used by the client, and therefore requires little effort on their end.

Updates:

• the simplest solution is to use the paypal donate SDK to add a donate button to the website. When clicked the user can authenticate and submit the donation via paypal in a popup over the BPR site. https://developer.paypal.com/sdk/donate/
• Investigate if there is a response from Paypal and if this can be used to save amounts to the db to track donations

[SPIKE] Review the Code with Jason webinar and make a list of refactor suggestions

Jason Swett reviewed this repo's code in a live webinar and made some suggestions for ways to refactor: https://www.youtube.com/watch?v=0Otyx30pfzY&t=2187s

This ticket is to watch that episode and make a list of suggested changes.

Reinstate application system test

If an adopter withdraws and removes an application from their applications list, staff will be able to reinstate this application in case the adopter accidentally withdrew. This reinstate checkbox has an information icon that provides details when clicked, using javascript.

This system test should test that the information box appears/disappears when this info icon is clicked.

Acceptance Criteria:

• all tests pass
• test checks the information box appears on icon click
• test checks the information box disappears on icon click

Look for and resolve N+1 queries

I probably wrote some N+1 queries in this code. As such, it would be good to review controllers, models and service objects for situations where this arises and determine if the issue can be resolved easily e.g., by eager loading data instead. See this article for context.

This would be a great issue to take on related to database query optimization - a common question in rails interviews :)

/donate page - push the footer to the bottom

See the /donate route and look at the footer - it is not pushed to the bottom as it should be.

Criteria:

• push footer to the bottom

Create Donations flow

on /successes we will have a Paypal donate button that uses the Paypal SDK to generate a popup to authenticate and complete the donation via paypal. This transaction will return an object via the SDK script. This object contains information that should be persisted so we can track donations over time.

Subtasks

• - Add Paypal SDK button to view
• - Style button
• - Create Donations controller with a #create action to save the donation information
• - Create Donations Table
• - Add AJAX call in onComplete controller (save $ value and currency if transaction is complete, otherwise discard)
• - Test all components work with Paypal sanbox accounts
• - Update SDK button for production - env and fetch url
• - Update SDK onComplete to make request to production instead of dev URL
• - Deploy and run rails db:migrate
• - Add ENV vars to app and Heroku
• - Test small donation with real card
• - Make sure the button loads on page and does not require refresh
• - Add integration tests for this flow
• - Add a task for active admin access to donations

Tests for User Account Settings Page

test/integration/user_account_test.rb
This file tests the /users/edit route (Dashboard > Account Settings in UI) and could do with some additional tests to ensure the form is working as intended to edit user settings.

Acceptance Criteria

• Integration tests are added to the above file, and pass
• Tests check the form functionality works for setting a new password, setting a new first and last name.

Clickable pins and random values to locations

I copied the repo and got it running on my local environment to experiment adding clickable pins and meta data. Here is the preview. I might open a PR after this. #144

---

My question is, the reason to apply random values to the duplicate locations (#134) because if the same person adopted multiple pets it's nearly impossible to individually display them on the map - they overlay each other, is that right or something else? @kasugaijin

Refactor the attachments controller

Currently there are no before_action filters to delete an active storage attachment (used for dog images). This flow needs to have a filter put in place (check it's a verified staff ) before allowing purging of an attachment.

Tests should also be added to org dogs test to confirm the filter works i.e., test shows that anyone who is not a verified staff in the same org as the dog should not be able to delete an attachment.

Add meta data to Google Maps Api pins

To be completed only once this issue is complete #134

As a user viewing the Google Map pins on the /succeses page I should be able to click a pin and retrieve metadata about the dog.

metadata should include at minimum:

• name
• breed

Also explore adding an image.