enable disable all agents within a scenario #1506

Jngai · 2016-05-16T20:00:02Z

Enabling or Disabling all agents within a scenario by updating all agents :disabled status. Showing a button and popping up a modal (partially inspired and code reused from the modal in action menu).

cantino · 2016-05-17T04:25:58Z

app/controllers/scenarios_controller.rb

+    @agents = @scenario.agents
+
+    respond_to do |format|
+      if @agents.update_all disabled: params[:scenario]


Wouldn't this be params[:scenario][:disabled] or params[:disabled]?

Jngai · 2016-05-17T21:33:52Z

app/controllers/scenarios_controller.rb

+    @scenario = current_user.scenarios.find(params[:id])
+
+    respond_to do |format|
+      if @scenario.agents.update_all disabled: params[:scenario][:disabled]


@dsander for some reason update_all isn't working........but I think it should. Would you know anything that scenario model is hooked up with that potentially make it fail? When I try it manually, it shows 2 records in the log but nothing was "committed".

It seems right. If you do p @scenario.agents, do you see the expected agents?

That's odd, if the SQL fails it should raise an exception. Can you paste the log output for a call of the method?

This is from better errors after I deliberately make it fail to access the console

>> params[:scenario][:disabled] => "true"

>> @scenario.agents #here disabled is false => #<ActiveRecord::Associations::CollectionProxy [#<Agents::TwitterSearchAgent id: 38, user_id: 1, options: {"search"=>"freebandnames", "expected_update_period_in_days"=>"2"}, type: "Agents::TwitterSearchAgent", name: "Twitter Search Agent", schedule: "every_12h", events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:33", updated_at: "2016-05-16 18:54:39", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "c48ff0f1e8fe5359e8e84601fe618ae5", deactivated: false>, #<Agents::TwitterPublishAgent id: 37, user_id: 1, options: {"expected_update_period_in_days"=>"10", "message"=>"{{text}}"}, type: "Agents::TwitterPublishAgent", name: "Twitter Publish Agent", schedule: nil, events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:16", updated_at: "2016-05-16 18:59:40", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "b0cc289b0c160778033718bdd3f8dd6a", deactivated: false>, #<Agents::TwitterFavorites id: 36, user_id: 1, options: {"username"=>"tectonic", "number"=>"10", "history"=>"100", "expected_update_period_in_days"=>"2"}, type: "Agents::TwitterFavorites", name: "Twitter Favorites", schedule: "every_12h", events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:02", updated_at: "2016-05-16 19:03:26", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "03c480dfb76dd1a254309bee864ec7ae", deactivated: false>]> >> @scenario.agents.update_all disabled: params[:scenario][:disabled] => 3 #does not show committed?

>> @scenario.agents #here disabled is still false => #<ActiveRecord::Associations::CollectionProxy [#<Agents::TwitterSearchAgent id: 38, user_id: 1, options: {"search"=>"freebandnames", "expected_update_period_in_days"=>"2"}, type: "Agents::TwitterSearchAgent", name: "Twitter Search Agent", schedule: "every_12h", events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:33", updated_at: "2016-05-16 18:54:39", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "c48ff0f1e8fe5359e8e84601fe618ae5", deactivated: false>, #<Agents::TwitterPublishAgent id: 37, user_id: 1, options: {"expected_update_period_in_days"=>"10", "message"=>"{{text}}"}, type: "Agents::TwitterPublishAgent", name: "Twitter Publish Agent", schedule: nil, events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:16", updated_at: "2016-05-16 18:59:40", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "b0cc289b0c160778033718bdd3f8dd6a", deactivated: false>, #<Agents::TwitterFavorites id: 36, user_id: 1, options: {"username"=>"tectonic", "number"=>"10", "history"=>"100", "expected_update_period_in_days"=>"2"}, type: "Agents::TwitterFavorites", name: "Twitter Favorites", schedule: "every_12h", events_count: 0, last_check_at: nil, last_receive_at: nil, last_checked_event_id: nil, created_at: "2016-05-11 21:25:02", updated_at: "2016-05-16 19:03:26", memory: {}, last_web_request_at: nil, keep_events_for: 0, last_event_at: nil, last_error_log_at: nil, propagate_immediately: false, disabled: false, service_id: 1, guid: "03c480dfb76dd1a254309bee864ec7ae", deactivated: false>]>

I don't see any errors in my rails log

I see a before_save call attach to the model which I commented out but it didn't go through. Also I recalled I can update a single record with update_attributes and had it successfully committed.

That looks totally fine to me, in the rails console you need to call @scenario.agents(true) or @scenario.reload; @scenario.agents to fetch the updated data from the database.
The log output also shows that sql UPDATE call was successful, are the agents not shown as disabled in the UI?

I tried reloading the scenario and the agents disabled status is still false.

The params is the same (with true status)

Parameters: {"utf8"=>"✓", "authenticity_token"=>"3t0nIpcahAGCZq6zueMZMnrc/fUATkEqRleoT6zB96lNZF9cIjzS3hT3fZ7tZPjmlW+DVv4JhSQChlme80DAsA==", "scenario"=>{"disabled"=>"true"}, "commit"=>"Yes", "id"=>"18"}

and no the agents did not showed disabled in the UI.

@scenario.agents(true) and @scenario.agents bring up records but is not updated.

never mind I fixed it, all I did was change true to 1 and false to 0 ........

Jngai · 2016-05-18T20:20:39Z

app/assets/javascripts/pages/scenario-show-page.js.coffee

+      $('#disabledfield').val 1
+    $('#enable_all').click ->
+      $('.modal-body').text 'Would you like to enable all agents?'
+      $('#disabledfield').val 0


Is it alright to have two functions within one instance property? I was looking at agent-show-page.js.coffee and it seems to be valid.

Jngai · 2016-05-19T17:34:20Z

app/controllers/scenarios_controller.rb

+      flash[:notice] = 'The agents has been successfully updated.'
+    end
+  end
+


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.

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) 😄

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.

Jngai · 2016-05-19T18:39:17Z

@cantino Will you code review this as well when you have time? I am having trouble with a bug that ci pointed out with null being passed to pg. I am using mysql at the moment. It should be a small bug and everything else is good too. Thank you.

dsander · 2016-05-19T22:26:17Z

app/controllers/scenarios_controller.rb

@@ -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]


The specs are failing on postgres because it handles boolean field differenctly then mysql. I think there are two options, either cast the param value to a ruby boolean here (!!params[:scenario][:disabled].to_ior params[:scenario][:disabled] == 'true' ? true : false) or change the hidden field to a hidden checkbox whose value rails automatically casts to a boolean.

Jngai · 2016-05-20T16:17:46Z

spec/controllers/scenarios_controller_spec.rb

+      expect(response).to redirect_to(scenario_path(scenarios(:bob_weather)))
+    end
+  end
+


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.

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

Jngai · 2016-05-20T17:39:40Z

@cantino This is good for a code review as well.

cantino · 2016-05-30T01:59:42Z

app/assets/javascripts/pages/scenario-show-page.js.coffee

@@ -0,0 +1,15 @@
+class @ScenarioShowPage
+  constructor:() ->
+    @changeModaltext()


Minor: changeModalText

cantino · 2016-05-30T02:02:45Z

Sorry for the delay.

I commented on a couple of minor things. Looking good!

cantino · 2016-06-05T00:32:36Z

app/controllers/scenarios_controller.rb

+    @scenario = current_user.scenarios.find(params[:id])
+
+    respond_to do |format|
+      if @scenario.agents.update_all disabled: (params[:scenario][:disabled]) == 'true' ? true : false)


I think there's an extra parentheses here.

You could just do:

@scenario.agents.update_all(disabled: params[:scenario][:disabled] == 'true')

Also, I don't believe that update_all can return false, so the else case will never get executed?

Jngai · 2016-06-07T21:47:06Z

@cantino Sorry I was away for something lately. I will find time for that other ticket that I have. I just put in two more fixes for your last comments, tested it locally and its good for another review. If all things go well I will squash it before we merge.

cantino · 2016-06-10T06:47:11Z

app/controllers/scenarios_controller.rb

+    @scenario = current_user.scenarios.find(params[:id])
+
+    respond_to do |format|
+      if @scenario.agents.update_all(disabled: params[:scenario][:disabled] == 'true')


I'd suggest just:

@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

No if, since the return value of update_all isn't meaningful.

Jngai · 2016-06-10T16:13:04Z

Hi @cantino I just removed the unnecessary if end statement. I think we should be good now.

cantino · 2016-06-14T05:36:30Z

Merged, thanks for the nice new feature @Jngai!

tentative work on enable disable all agents within a scenario

2e2be8d

Jngai mentioned this pull request May 16, 2016

Enhancement: Enable/Disable All Agents button in Scenario #1489

Closed

Jngai changed the title ~~tentative work on enable disable all agents within a scenario~~ enable disable all agents within a scenario May 16, 2016

cantino reviewed May 17, 2016
View reviewed changes

Judy Ngai added 3 commits May 17, 2016 14:07

more work with pr

1529c6f

more work

c314225

better names and save a line of code

b7684b9

Jngai reviewed May 17, 2016
View reviewed changes

Judy Ngai added 4 commits May 17, 2016 21:32

coffeescript to change modal text and hidden disabled value

dfbbf6b

redoing coffeescript class to save some code

90fcd6f

initing the function

c0aaa19

updated coffeescript

a644c92

Jngai reviewed May 18, 2016
View reviewed changes

Judy Ngai added 5 commits May 19, 2016 10:51

change text from specific modal, made names more specific

a673743

updates disabled test and rubocop bug

fae6c10

improved test and refactored method per rubocop warnings

a146fc8

switched from 0 to false

5895e76

switched from 0 to false and 1 to true

fbc5e62

Jngai reviewed May 19, 2016
View reviewed changes

fixed null bug in postgresql database

332106c

dsander reviewed May 19, 2016
View reviewed changes

Judy Ngai added 4 commits May 20, 2016 10:58

fixed ruby boolean bug

4a3bb4b

moved instances into let lazy load and use proper agent fixture

02200f3

putting in a different fix

6553c92

using existing membership and agent and another version of test

3e53dd8

Jngai reviewed May 20, 2016
View reviewed changes

switch back to 2 not 3 after rm an agent in yml

b1a1c49

Jngai closed this May 20, 2016

Jngai reopened this May 20, 2016

made test passed locally

7456400

Judy Ngai added 4 commits May 20, 2016 13:40

forgot to remove debuging pp

ab5a529

small spacing bug

39d38e9

trying to get rid of newline in file

8655fe1

typo not agent but scenario

9e85f8e

cantino reviewed May 30, 2016
View reviewed changes

minor changes to syntax and naming

ea48a9b

cantino reviewed Jun 5, 2016
View reviewed changes

Judy Ngai added 2 commits June 7, 2016 17:20

rm potential failure and changed update_all statement

a2ca396

fixing new line issue

95ad506

cantino reviewed Jun 10, 2016
View reviewed changes

removed unnecessary if else statement from method

6651006

cantino merged commit f57a3af into huginn:master Jun 14, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable disable all agents within a scenario #1506

enable disable all agents within a scenario #1506

Jngai commented May 16, 2016 •

edited

Loading

cantino May 17, 2016

Jngai May 17, 2016 •

edited

Loading

cantino May 18, 2016

dsander May 18, 2016

Jngai May 18, 2016 •

edited

Loading

dsander May 18, 2016

Jngai May 18, 2016

Jngai May 18, 2016 •

edited

Loading

Jngai May 18, 2016

cantino May 19, 2016

Jngai May 19, 2016 •

edited

Loading

dsander May 19, 2016

Jngai May 20, 2016

Jngai commented May 19, 2016 •

edited

Loading

dsander May 19, 2016

Jngai May 20, 2016

Jngai May 20, 2016 •

edited

Loading

Jngai commented May 20, 2016

cantino May 30, 2016

cantino commented May 30, 2016

cantino Jun 5, 2016

cantino Jun 5, 2016

Jngai commented Jun 7, 2016

cantino Jun 10, 2016

Jngai commented Jun 10, 2016 •

edited

Loading

cantino commented Jun 14, 2016

enable disable all agents within a scenario #1506

enable disable all agents within a scenario #1506

Conversation

Jngai commented May 16, 2016 • edited Loading

Choose a reason for hiding this comment

Jngai May 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai May 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai May 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai May 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai commented May 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai May 20, 2016 • edited Loading

Choose a reason for hiding this comment

Jngai commented May 20, 2016

Choose a reason for hiding this comment

cantino commented May 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jngai commented Jun 7, 2016

Choose a reason for hiding this comment

Jngai commented Jun 10, 2016 • edited Loading

cantino commented Jun 14, 2016

Jngai commented May 16, 2016 •

edited

Loading

Jngai May 17, 2016 •

edited

Loading

Jngai May 18, 2016 •

edited

Loading

Jngai May 18, 2016 •

edited

Loading

Jngai May 19, 2016 •

edited

Loading

Jngai commented May 19, 2016 •

edited

Loading

Jngai May 20, 2016 •

edited

Loading

Jngai commented Jun 10, 2016 •

edited

Loading