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 phase filter to placements search #1211

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dp-daly
Copy link
Contributor

@dp-daly dp-daly commented Nov 27, 2024

Context

This design change allows provider users to find placements in Primary or Secondary more easily.

Changes proposed in this pull request

  • Add a phase filter option to the filter menu on the placements index.
  • Add an accompanying filter tag when a phase filter is selected.
  • Add functionality that changes other filter options depending on whether a phase filter has been selected (see expected behaviour below).
  • Add system spec for new phase filter.
  • Update filter form spec to include tests for expected phase filter behaviour.

Guidance to review

  • Toggle the phase filter options and test it against the below expected behaviour.
  • Review new system spec and changes to filter form spec.

When no checkboxes are selected (default state)

  • Search results: Show both primary and secondary placements in the results
  • “Subject” filter: Show both primary and secondary subjects in the list of options
  • “Primary year group” filter: Show this filter
  • “School” filter: Show both primary and secondary schools in the list of options

When only Primary is selected

  • Search results: Only show placements for primary subjects
  • “Subject” filter: Only show primary subjects in the list of options
  • “Primary year group” filter: Show this filter
  • “School” filter: Exclude secondary schools from the list of options

When only Secondary is selected

  • Search results: Only show placements for secondary subjects
  • “Subject” filter: Only show secondary subjects in the list of options
  • “Primary year group” filter: Hide this filter
  • “School” filter: Exclude primary schools from the list of options

When both Primary and Secondary are selected

Same behaviour as when no checkboxes are selected (default state) ☝️

Link to Trello card

https://trello.com/c/oCkrtGGU/951-add-a-phase-filter-to-placements-search

Screenshots

Screen.Recording.2024-11-27.at.16.53.24.mov

@dp-daly dp-daly requested a review from a team as a code owner November 27, 2024 16:56
@dp-daly dp-daly self-assigned this Nov 27, 2024
@dp-daly dp-daly added the deploy A Review App will be created for PRs with this label label Nov 28, 2024
Copy link

@dp-daly dp-daly force-pushed the dpd/add-phase-filter-to-placements-search branch 2 times, most recently from 3a38b25 to d1d9ebf Compare November 28, 2024 15:18
@@ -14,6 +14,8 @@ def index
@pagy, @placements = pagy(placements.merge(query))
@placements = @placements.decorate

filter_subjects_by_phase
filter_schools_by_phase
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather and putting this here. Consider putting this as part of the schools_scope method.

Copy link
Contributor Author

@dp-daly dp-daly Dec 5, 2024

Choose a reason for hiding this comment

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

@JamieCleare2525 I haven't been able to incorporate this without affecting the initialization of @schools across the app 🤔 I thought this would work but it didn't:

  def schools_scope
    if filter_params[:only_partner_schools].present?
      @schools = @provider.partner_schools
    else
      @schools = Placements::School.all
    end
    
    filter_schools_by_phase
  end

I get 58 instances of an error saying that .order_by_name can't act on nil.

@dp-daly dp-daly force-pushed the dpd/add-phase-filter-to-placements-search branch from d1d9ebf to 6dee289 Compare December 5, 2024 09:37
@dp-daly dp-daly requested a review from a team as a code owner December 5, 2024 09:37
@dp-daly dp-daly force-pushed the dpd/add-phase-filter-to-placements-search branch from 6dee289 to 57d30e0 Compare December 17, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy A Review App will be created for PRs with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants