Skip to content

Add "Give" search pill - #802

Open
jdelagarzaf wants to merge 5 commits into
add-inkind-donationsfrom
give-search-pill
Open

Add "Give" search pill#802
jdelagarzaf wants to merge 5 commits into
add-inkind-donationsfrom
give-search-pill

Conversation

@jdelagarzaf

Copy link
Copy Markdown
Collaborator

Context

What changed

How to test it

References

ClickUp ticket

@jdelagarzaf

jdelagarzaf commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Hey Nathaly, great work.
There is just one blocker issue: the app/queries/locations/filter_query.rb file now returns scope at the all function, when it used to return the result of opened_on_weekends that appended scope. This breaks it and loses the filtering for organizations open on weekends.
Simple fix, just make sure to store the result of opened_on_weekends in scope before calling scope at the end, that is, just add "scope =" at the line above it returning.

BEFORE

scope = by_give(scope, params[:give])
scope = opened_now(scope, params[:open_now])
opened_on_weekends(scope, params[:open_weekends])
scope 

AFTER

scope = by_give(scope, params[:give])
scope = opened_now(scope, params[:open_now])
scope = opened_on_weekends(scope, params[:open_weekends])
scope 

@jdelagarzaf

Copy link
Copy Markdown
Collaborator Author

Here's another bug: Currently, by_give matches only in_kind_donation_link. Since you worked on the back before Hazel finished hers, there was a new field added. Make sure it matches both the donation link presence as well as the donation items being present.

Another issue, the "Create Search Alert" button at the top of the list of filtered organizations, it is rendered as disabled when a "Give" filter is selected, when it shouldnt. This is becasue the file searches_helper.rb:6-27 still doesn't know about "give".

Regarding your questions about filters, I believe they should be OR but rank the first options with the highest amount of AND matches. This is especially useful if you end up adding filter pills for each donation item from the list, but confirm with Stephanie if this is a wanted feature or not.

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.

2 participants