-
Notifications
You must be signed in to change notification settings - Fork 27
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
[#162225] Multi-Facility - Duplicates being created #4189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! Left a couple small comments
app/forms/add_to_order_form.rb
Outdated
@@ -31,6 +31,7 @@ def save | |||
|
|||
if @original_order.facility.id == @facility_id | |||
add_to_order! | |||
return true | |||
end | |||
|
|||
return true unless SettingsHelper.feature_on?(:cross_core_projects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer to wrap the feature flagged changes in a conditional, instead of using a guard clause:
if SettingsHelper.feature_on?(:cross_core_projects)
if order_for_selected_facility.nil?
create_cross_core_project_and_add_order!
else
add_to_order!
end
end
... then we can still git true
on line 45.
This would be easier to read and clearer what needs to happen when we are ready to remove the flag.
app/forms/add_to_order_form.rb
Outdated
create_cross_core_project_and_add_order! | ||
else | ||
add_to_order! | ||
if SettingsHelper.feature_on?(:cross_core_projects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LeticiaErrandonea maybe this should be elsif
?
* Fix bug and add spe * Solve one accessibility problem * Improve code * Fix bug * Use elsif
Release Notes
Additional Context
Add a spec to add to facility order using Feature Flag.
New spec failed with old code as it found two "Make a reservation" buttons.
Accessibility