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

355 filtering adopter applications #373

Merged

Conversation

phonghpham
Copy link
Contributor

@phonghpham phonghpham commented Dec 17, 2023

🔗 Issue

resolves #355

✍️ Description

Allow staff to filter adopter applications by pet name, applicant name, and application status.

📷 Screenshots/Demos

Alta.Pet.Rescue._._Pet.Rescue_.-.16.December.2023.mp4

@phonghpham phonghpham force-pushed the 355-filtering-adopter-applications branch 3 times, most recently from dc99314 to 7b8ab9c Compare December 17, 2023 02:31
@kasugaijin
Copy link
Collaborator

kasugaijin commented Dec 20, 2023

This looks great! Unless someone beats me to it, I will review this later this week.
Couple tests are failing in the pipeline. Are they failing locally?

@phonghpham phonghpham force-pushed the 355-filtering-adopter-applications branch from 12a46de to 3dca156 Compare December 20, 2023 03:13
@kasugaijin
Copy link
Collaborator

I played around and the search looks good.
Can we add the Pet name in place of the word 'Applications' on each card displaying applications?
image

Also, is this happening for you? If I go to a pet, none of the tabs are rendering. Have you merged main branch in? I don't think I see it in your commits. That might fix it. There should be partials rendered here for each tab...none rendering.
image

but rather as local var passed in from search
results partial. Since applications is also passed
from search results partial, use that.
by integrating Ransack into index.
- Update @Pets variable to better reflect data being handled
filtering still work. Tested this locally, and without this seemingly
superfluous check, only application status can be filtered.
@phonghpham phonghpham force-pushed the 355-filtering-adopter-applications branch from 84f3247 to 4567e77 Compare December 21, 2023 05:13
@phonghpham
Copy link
Contributor Author

@kasugaijin, you're right, after I rebased off main and made a tweak all tabs are now rendering in pet show view. Tests passing locally and updated Applications to reflect pet name.

Copy link
Collaborator

@kasugaijin kasugaijin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! thank you for taking it on @phonghpham

@kasugaijin kasugaijin merged commit ec3af56 into rubyforgood:main Dec 22, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As a staff I can filter adopter applications by pet name, applicant name, application status
2 participants