FFS-2336: First sketch of multi-provider search #442
Conversation
| def initialize(client_agency_id) | ||
| set_pinwheel(client_agency_id) | ||
| set_argyle(client_agency_id) | ||
| end |
There was a problem hiding this comment.
Need a way more elegant approach to this, not sure yet
There was a problem hiding this comment.
These services should implement a common interface somehow but I'm not sure what that'd be yet.
| def search(query = "") | ||
| [ | ||
| *@argyle.items(query)["results"].map do |result| | ||
| PROVIDER_RESULT.new(provider_name: :argyle, provider_options: { response_type: result["kind"], provider_id: result["id"] }, name: result["name"], logo_url: result["logo_url"]) | ||
| end, | ||
| *@pinwheel.fetch_items(q: query)["data"].map do |result| | ||
| PROVIDER_RESULT.new(provider_name: :pinwheel, provider_options: { response_type: result["response_type"], provider_id: result["id"] }, name: result["name"], logo_url: result["logo_url"]) | ||
| end | ||
| ] | ||
| end |
There was a problem hiding this comment.
This is pretty bad because it doesn't deal with error failures or how to handle pagination!
allthesignals
force-pushed
the
wmg/2336-implement-multi-provider-search
branch
from
4354c1e to
d5c0d09
Compare
| def provider_search(query = "") | ||
| ProviderSearchService.new(@cbv_flow.site_id).search(query) | ||
| end |
There was a problem hiding this comment.
What are the memory implications of instantiating this every time? It's only really used for looking up some values specific to the partner agency configuration...
There was a problem hiding this comment.
I guess it's only used in once place so it doesn't really matter right now
There was a problem hiding this comment.
Memory implications are probably not too bad, since GC should clear it out automatically after the method runs. There is a tiny performance hit in allocating and then GCing it, but whatever, not worth worrying about.
| class PinwheelAdapter | ||
| def initialize(environment) | ||
| @pinwheel = PinwheelService.new(environment) | ||
| end | ||
|
|
||
| def query(query) | ||
| @pinwheel.fetch_items(q: query)["data"].map do |result| | ||
| PROVIDER_RESULT.new( | ||
| provider_name: :pinwheel, | ||
| provider_options: { | ||
| response_type: result["response_type"], | ||
| provider_id: result["id"] | ||
| }, | ||
| name: result["name"], | ||
| logo_url: result["logo_url"] | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| class ArgyleAdapter | ||
| def initialize(environment) | ||
| @argyle = ArgyleService.new(environment) | ||
| end | ||
|
|
||
| def query(query) | ||
| @argyle.items(query)["results"].map do |result| | ||
| PROVIDER_RESULT.new( | ||
| provider_name: :argyle, | ||
| provider_options: { | ||
| response_type: result["kind"], | ||
| provider_id: result["id"] | ||
| }, | ||
| name: result["name"], | ||
| logo_url: result["logo_url"] | ||
| ) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Now after doing all of this I wonder if it makes sense just to have the services themselves assume a common interface. I don't know if you'd enforce it. There's no reason why PROVIDER_RESULT can't be used in the service classes themselves? 🤔
allthesignals
force-pushed
the
wmg/2336-implement-multi-provider-search
branch
from
ecd2041 to
d5c0d09
Compare
allthesignals
changed the title
|
Looking good. What changes were you planning to make in response to reviewing it at Eng Time earlier today? |
…ow-use-new-database * origin/main: FFS-2336: First sketch of multi-provider search (#442)
Ticket
Resolves FFS-2336.
Changes
Integrates Argyle, implements a common multi-search interface, used in employer search
Context for reviewers
Very rough draft of the provider search class and the approach to supporting multiple providers. There are nuances around SiteConfig, and how the provider (specifically pinwheel) is set up and accessed in controllers that I haven't figured out yet.
Acceptance testing
:alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!)