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

enable disable all agents within a scenario #1506

Merged
merged 28 commits into from
Jun 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2e2be8d
tentative work on enable disable all agents within a scenario
May 16, 2016
1529c6f
more work with pr
May 17, 2016
c314225
more work
May 17, 2016
b7684b9
better names and save a line of code
May 17, 2016
dfbbf6b
coffeescript to change modal text and hidden disabled value
May 18, 2016
90fcd6f
redoing coffeescript class to save some code
May 18, 2016
c0aaa19
initing the function
May 18, 2016
a644c92
updated coffeescript
May 18, 2016
a673743
change text from specific modal, made names more specific
May 19, 2016
fae6c10
updates disabled test and rubocop bug
May 19, 2016
a146fc8
improved test and refactored method per rubocop warnings
May 19, 2016
5895e76
switched from 0 to false
May 19, 2016
fbc5e62
switched from 0 to false and 1 to true
May 19, 2016
332106c
fixed null bug in postgresql database
May 19, 2016
4a3bb4b
fixed ruby boolean bug
May 20, 2016
02200f3
moved instances into let lazy load and use proper agent fixture
May 20, 2016
6553c92
putting in a different fix
May 20, 2016
3e53dd8
using existing membership and agent and another version of test
May 20, 2016
b1a1c49
switch back to 2 not 3 after rm an agent in yml
May 20, 2016
7456400
made test passed locally
May 20, 2016
ab5a529
forgot to remove debuging pp
May 20, 2016
39d38e9
small spacing bug
May 20, 2016
8655fe1
trying to get rid of newline in file
May 20, 2016
9e85f8e
typo not agent but scenario
May 20, 2016
ea48a9b
minor changes to syntax and naming
Jun 1, 2016
a2ca396
rm potential failure and changed update_all statement
Jun 7, 2016
95ad506
fixing new line issue
Jun 7, 2016
6651006
removed unnecessary if else statement from method
Jun 10, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions app/assets/javascripts/pages/scenario-show-page.js.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class @ScenarioShowPage
constructor:() ->
@changeModalText()

changeModalText: () ->
$('#disable-all').click ->
$('#enable-disable-agents .modal-body').text 'Would you like to disable all agents?'
$('#scenario-disabled-value').val 'true'
$('#enable-all').click ->
$('#enable-disable-agents .modal-body').text 'Would you like to enable all agents?'
$('#scenario-disabled-value').val 'false'

$ ->
Utils.registerPage(ScenarioShowPage, forPathsMatching: /^scenarios/)

10 changes: 10 additions & 0 deletions app/controllers/scenarios_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ def update
end
end

def enable_or_disable_all_agents
@scenario = current_user.scenarios.find(params[:id])

@scenario.agents.update_all(disabled: params[:scenario][:disabled] == 'true')
respond_to do |format|
format.html { redirect_to @scenario, notice: 'The agents in this scenario have been successfully updated.' }
format.json { head :no_content }
end
end

Copy link
Contributor Author

@Jngai Jngai May 19, 2016

Choose a reason for hiding this comment

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

I whittled this down to a bit because of rubocop assignment branch error (method does too much). I think this is more akin to the destroy method below it than the update method above it. The likelihood that this will fail is on the low side.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't worry about the code complexity offenses of rubocop too much, in my suggestion #1505 I disabled them completely because the default values are really low and almost impossible to achieve in a rails project (at least in my experience) 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if we are going to use rubocop from now on to check the code style. I actually grab the yml file from your pr. I will put what I had back in later.

def destroy
@scenario = current_user.scenarios.find(params[:id])
@scenario.destroy_with_mode(params[:mode])
Expand Down
19 changes: 19 additions & 0 deletions app/views/scenarios/_enable_agents_modal.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<div id="enable-disable-agents" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="enableAgentLabel" aria-hidden="true">
<div class="modal-dialog modal-sm">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>
<h4 class="modal-title">Confirm</h4>
</div>
<div class="modal-body">
</div>
<div class="modal-footer">
<%= form_for(scenario, as: :scenario, url: enable_or_disable_all_agents_scenario_path(scenario), method: 'PUT') do |f| %>
<%= f.hidden_field :disabled, value: '', id: "scenario-disabled-value" %>
<%= f.button 'No', class: 'btn btn-default', 'data-dismiss' => 'modal' %>
<%= f.submit 'Yes', class: 'btn btn-primary' %>
<% end %>
</div>
</div>
</div>
</div>
3 changes: 3 additions & 0 deletions app/views/scenarios/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@
<% end %>
<%= link_to icon_tag('glyphicon-share-alt') + ' Share', share_scenario_path(@scenario), class: "btn btn-default" %>
<%= link_to icon_tag('glyphicon-trash') + ' Delete', '#', data: { toggle: 'modal', target: "#confirm-scenario-deletion-#{@scenario.id}"}, class: "btn btn-default" %>
<%= link_to icon_tag('glyphicon-play') + 'Enable all Agents', '#', data: { toggle: 'modal', target: "#enable-disable-agents"}, class: "btn btn-default", id: "enable-all" %>
<%= link_to icon_tag('glyphicon-pause') + 'Disable all Agents', '#', data: { toggle: 'modal', target: "#enable-disable-agents"}, class: "btn btn-default", id: "disable-all" %>
</div>
</div>
</div>
</div>
<%= render 'scenarios/confirm_deletion_modal', scenario: @scenario %>
<%= render 'scenarios/enable_agents_modal', scenario: @scenario %>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
member do
get :share
get :export
put :enable_or_disable_all_agents
end

resource :diagram, :only => [:show]
Expand Down
9 changes: 9 additions & 0 deletions spec/controllers/scenarios_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ def valid_attributes(options = {})
end
end

describe 'PUT enable_or_disable_all_agents' do
it 'updates disabled on all agents in a scenario for the current user' do
@params = {"scenario"=>{"disabled"=>"true"}, "commit"=>"Yes", "id"=> scenarios(:bob_weather).id}
put :enable_or_disable_all_agents, @params
expect(agents(:bob_rain_notifier_agent).disabled).to eq(true)
expect(response).to redirect_to(scenario_path(scenarios(:bob_weather)))
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure why this test isn't passing. The params are in hash and it should be getting put correctly. I also tried post. Locally it is fine but the test just isn't passing.

Copy link
Contributor Author

@Jngai Jngai May 20, 2016

Choose a reason for hiding this comment

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

Never mind I fixed it. I just need to modify params a bit and is straight from my log.

describe "DELETE destroy" do
it "destroys only Scenarios owned by the current user" do
expect {
Expand Down