Parked client download #816
Conversation
| @@ -368,85 +368,6 @@ def active_in_match? | |||
| active_matches.any? | |||
| end | |||
|
|
|||
| def available_as_candidate_for_any_route? | |||
There was a problem hiding this comment.
This was moved into the Availability concern, where similar methods already existed.
| # TODO: Disabled in the nightly processing, remove? | ||
| def self.ensure_availability | ||
| self.available.each(&:ensure_availability) | ||
| end | ||
|
|
||
| def ensure_availability | ||
| self.class.transaction do | ||
| unavailable_as_candidate_fors.destroy_all | ||
| unavailable_routes.each do |route| | ||
| self.make_unavailable_in match_route: route | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
This was unused code
| @@ -216,11 +216,11 @@ def confidential? | |||
| # remove voucher | |||
| # remove self (opportunity) | |||
|
|
|||
| def stop_matches_and_remove_entire_history_from_existance! | |||
| def stop_matches_and_remove_entire_history_from_existence! | |||
There was a problem hiding this comment.
This had a typo, but as far as I could tell isn't used anywhere, probably a manual mechanism for data cleanup?
| MatchRoutes::Base.active.each do |route| | ||
| matches = ClientOpportunityMatch.active.on_route(route).where(client_id: unavailable_client_ids).group_by(&:client_id) | ||
| UnavailableAsCandidateFor.for_route(route).where(client_id: matches.keys).find_each do |record| | ||
| record.update(reason: 'Active Match', user_id: record.versions&.first&.user_id, match_id: matches[record.client_id].first.id) |
There was a problem hiding this comment.
is matches[record.client_id] always present?
There was a problem hiding this comment.
Should be. matches is generated using unavailable_client_ids so the client ids should be a subset of the overall parked clients (those on a particular route.) And every match and parked record will have a client_id
| @@ -83,15 +80,13 @@ def pending | |||
|
|
|||
| def confirmed | |||
| Notifications::MatchSuccessConfirmedDevelopmentOfficer.create_for_match! match | |||
| match.succeeded! | |||
| match.succeeded!(user: @dependencies.try(:[], :user)) | |||
There was a problem hiding this comment.
Not sure but maybe it could be worth it to put this repeated code in a method on the base class?
# match.succeeded!(user: user)
def user
@dependencies.try(:[], :user)
endThere was a problem hiding this comment.
Thanks, that's cleaner for sure.
| %tr | ||
| %td | ||
| = link_to_if can_view_all_clients?, client.last_name, client_path(client) | ||
| %td= link_to_if can_view_all_clients?, client.first_name, client_path(client) | ||
| %td= client.unavailable_as_candidate_fors.merge(UnavailableAsCandidateFor.for_route(@route))&.first&.expires_at&.to_date | ||
| %td= parked&.expires_at&.to_date | ||
| %td= parked&.user&.name_with_email |
There was a problem hiding this comment.
Not sure we can, we're loading parked on line 36 as an N+1, so preloading the user would already be another query. Should be fine, these are paginated, and those queries should be quick.
app/models/concerns/availability.rb
Outdated
| opportunity.update(available_candidate: true) | ||
| end | ||
| # Delete any non-active open matches | ||
| client_opportunity_matches.open.each(&:delete) |
There was a problem hiding this comment.
these are ClientOpportunityMatch records? It seems like calling delete rather than destroy could leave orphaned records since delete won't trigger callbacks
| end | ||
|
|
||
| def self.download_columns |
There was a problem hiding this comment.
This is #6133 right? Some differences here from the spec (missing Warehouse ID) but maybe that's okay?
There was a problem hiding this comment.
Warehouse ID is actually Remote ID but it needs to be paired with Remote Source because the clients come from multiple sources. So that should suffice.
Co-authored-by: Theron Toomey <[email protected]>
Co-authored-by: Theron Toomey <[email protected]>
Please squash merge this PR
Description
This:
Type of change
Checklist before requesting review