-
Notifications
You must be signed in to change notification settings - Fork 10
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
fixed managment of optimizations #129
Conversation
def new | ||
@optimization = Optimization.new | ||
authorize @optimization | ||
optim_num = Optimization.owned_by(current_user).size | ||
if optim_num > 19 |
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.
Avoid the magic-number effect by using a constant MAX_OPTIM_NUMBER = 20
(is it not 20 for allowing 20 optims?). Ideally this constant should be defined in Optimization
class as it has to be used also in the api\v1\optimization_controller.rb.
By the way, could you also implement the optim limit in that file for the JSON REST api? May be you can even add a test for this: create 20 optims and check for an error when trying to create the 21fst.
def new | ||
@optimization = Optimization.new | ||
authorize @optimization | ||
optim_num = Optimization.owned_by(current_user).size | ||
if optim_num > 19 | ||
redirect_to optimizations_url, notice: "You own too many optimizations (#{optim_num}), you must delete some before creating new ones" |
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 you can use alert:
instead of notice:
, it changes the flash message color.
if File.exist?(path) | ||
send_file(path) | ||
else | ||
redirect_to optimization_path(optim), notice: "There isn't a log file" |
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.
alert:
instead of notice:
end | ||
end | ||
if errors != "" | ||
redirect_to edit_optimization_path(@optimization), notice: "Optimization update failed, #{errors}" |
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.
here and just below alert:
instead of notice:
No description provided.