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 CbvFlows model and controller for flow logic (FFS-679) #7

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

tdooner
Copy link
Contributor

@tdooner tdooner commented Apr 26, 2024

  • Store the Argyle User ID for the current user in a model called
    CbvFlow.
  • Create a CbvFlow controller to handle routing between those pages.
    This controller may get big; we can break it up if it does.
  • Add a bin/update script to run after you git pull to make sure you
    have all the new dependencies and everything.
  • Add some light testing of each page separately

* Store the Argyle User ID for the current user in a model called
  `CbvFlow`.
* Create a CbvFlow controller to handle routing between those pages.
  This controller may get big; we can break it up if it does.
* Add a `bin/update` script to run after you `git pull` to make sure you
  have all the new dependencies and everything.
* Add some light testing of each page separately
The filename needs to end with _spec.rb for Rspec to discover and run
it.
def fetch_and_store_argyle_token
return session[:argyle_user_token] if session[:argyle_user_token].present?

raise "ARGYLE_API_TOKEN environment variable is blank. Make sure you have the .env.local from 1Password." if ENV['ARGYLE_API_TOKEN'].blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually we could have a config system that validates this stuff elsewhere


raise "ARGYLE_API_TOKEN environment variable is blank. Make sure you have the .env.local from 1Password." if ENV['ARGYLE_API_TOKEN'].blank?

res = Net::HTTP.post(URI.parse(USER_TOKEN_ENDPOINT), "", {"Authorization" => "Basic #{ENV['ARGYLE_API_TOKEN']}"})
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where we'd call @GeorgeCodes19' service class (or a private wrapper method for it).

@@ -1,8 +1,10 @@
<h1>Hello, Matt</h1>
<h1>Hello</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

Bye, Matt!


scope '/cbv', as: :cbv_flow do
get '/entry' => 'cbv_flows#entry'
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: As far as naming goes, is the cbv_ prefix redundant? I don't feel strongly about it, it's a convention in Login, too (idp_{object}).

def change
create_table :cbv_flows do |t|
t.string :case_number
t.string :argyle_user_id
Copy link
Contributor

Choose a reason for hiding this comment

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

In a later PR, we may want to store the Argyle-supplied JWT? Although that may be a case of "inappropriate intimacy" between our domain modeling and the 3rd parties' architecture...

Edit: actually I think that is managed by Argyle in its own cookie. Will have to investigate...

Basically the presence of an Argule cookie would affect how we determine flow state.

Additionally, At the moment the user is Linked, we are provided with an account_id that we'd use to query for paystubs in the next step. So maybe that context info is captured in the CbvFlow.

end
helper_method :next_path

def fetch_and_store_argyle_token
Copy link
Contributor

Choose a reason for hiding this comment

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

Future service class stuff

root_url
end
end
helper_method :next_path
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting I'm not familiar with this. Does this expose the method for use in the templates?


private

def set_cbv_flow
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be on the model? Maybe returns a symbol or something. I guess my concern is it works by inference - we infer the step based on the presence of an id. Wondering if there's a way to capture that behavior closer to a model, without the controller context

bin/update Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Great thanks!

Comment on lines +4 to +13
def stub_environment_variable(variable, value, &block)
previous_value = ENV[variable]
ENV[variable] = value
block.call
ENV[variable] = previous_value
end

around do |ex|
stub_environment_variable("ARGYLE_API_TOKEN", "foobar", &ex)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Slick!

# Conflicts:
#	app/controllers/providers_controller.rb
#	config/routes.rb
@allthesignals
Copy link
Contributor

allthesignals commented Apr 29, 2024

I tried it out on my machine, I don't think the Javascript is being picked up anymore! So I'd hold off on merging.

Fixed! Tested it on my machine, the Link stuff works.

@allthesignals allthesignals merged commit 57f0683 into main Apr 29, 2024
6 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.

None yet

2 participants