Skip to content

LG-11688 Enforce the IdV available rule in a concern#9665

Merged
jmhooper merged 1 commit intomainfrom
jmhooper-idv-availability
Nov 28, 2023
Merged

LG-11688 Enforce the IdV available rule in a concern#9665
jmhooper merged 1 commit intomainfrom
jmhooper-idv-availability

Conversation

@jmhooper
Copy link
Contributor

We have been discussing feature flags that can be toggled while the applicaiton is running. This would be an issue for the feature flags that drive FeatureManagement#idv_available?. That method is used in the routes file to handle routing for IdV routes when IdV is unavailable. This means that to change those feature flags while the app is running carries a requirement to redraw routes.

This commit makes a change to move equivalent logic to the routes file to the Idv::Availability concern. This concern is then included in all of the controller that were previously affected by the rule in the routes file.

We have been discussing feature flags that can be toggled while the applicaiton is running. This would be an issue for the feature flags that drive `FeatureManagement#idv_available?`. That method is used in the routes file to handle routing for IdV routes when IdV is unavailable. This means that to change those feature flags while the app is running carries a requirement to redraw routes.

This commit makes a change to move equivalent logic to the routes file to the `Idv::Availability` concern. This concern is then included in all of the controller that were previously affected by the rule in the routes file.

changelog: Internal, IdV Availability, IdV availability is computed in a concern instead of the routes file
@jmhooper jmhooper requested a review from matthinz November 27, 2023 21:36
@jmhooper
Copy link
Contributor Author

I opted for a concern that we can explicitly include in controllers instead of including it in a shared concern for 2 reasons:

  1. It makes it easy to see which controllers are impacted without having to worry about it being implicit with the inclusion of another concern
  2. Nested concerns can lead to issues with before actions in the included block which no one wants to deal with

Copy link
Contributor

@matthinz matthinz left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -310 to -314
if !FeatureManagement.idv_available?
# IdV has been disabled.
match '/*path' => 'unavailable#show', via: %i[get post]
end

Copy link
Contributor

Choose a reason for hiding this comment

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

:bless:

Comment on lines -65 to -69

# Configuration / vendor status changes can effect Rails routing tables.
# Force routes to be reloaded when we've modified configuration.
Rails.application.reload_routes!
end
Copy link
Contributor

Choose a reason for hiding this comment

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

also :bless:

@jmhooper jmhooper merged commit f8e596e into main Nov 28, 2023
@jmhooper jmhooper deleted the jmhooper-idv-availability branch November 28, 2023 13:36
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.

3 participants