Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Zhe (Joe) Wang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/#review148775
---


Ship it!




Ship It!

- Zhe (Joe) Wang


On Sept. 9, 2016, 10:07 p.m., Vivek Ratnavel Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51540/
> ---
> 
> (Updated Sept. 9, 2016, 10:07 p.m.)
> 
> 
> Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.
> 
> 
> Bugs: Ambari-18281
> https://issues.apache.org/jira/browse/Ambari-18281
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Add an option to Enable/ Disable alert notifications in Manage Notifications 
> popup.
> 
> 
> Diffs
> -
> 
>   
> ambari-web/app/controllers/main/alerts/alert_definitions_actions_controller.js
>  f3f0387 
>   ambari-web/app/styles/modal_popups.less a2343e0 
> 
> Diff: https://reviews.apache.org/r/51540/diff/
> 
> 
> Testing
> ---
> 
> Verified manually.
> Ambari-web unit tests pass.
> 30014 tests complete (26 seconds)
> 154 tests pending
> 
> 
> File Attachments
> 
> 
> v1
>   
> https://reviews.apache.org/media/uploaded/files/2016/08/31/ad971a3a-3430-4caa-9e7f-df60cc75872c__AMBARI-18281.v1.patch
> 
> 
> Thanks,
> 
> Vivek Ratnavel Subramanian
> 
>

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Vivek Ratnavel Subramanian]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/
---

(Updated Sept. 9, 2016, 10:07 p.m.)


Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.


Changes
---

Changes made to fix AMBARI-18306.
Verified manually.
Ambari-web unit tests pass
30182 tests complete (26 seconds)
151 tests pending


Bugs: Ambari-18281
https://issues.apache.org/jira/browse/Ambari-18281


Repository: ambari


Description
---

Add an option to Enable/ Disable alert notifications in Manage Notifications 
popup.


Diffs (updated)
-

  
ambari-web/app/controllers/main/alerts/alert_definitions_actions_controller.js 
f3f0387 
  ambari-web/app/styles/modal_popups.less a2343e0 

Diff: https://reviews.apache.org/r/51540/diff/


Testing
---

Verified manually.
Ambari-web unit tests pass.
30014 tests complete (26 seconds)
154 tests pending


File Attachments


v1
  
https://reviews.apache.org/media/uploaded/files/2016/08/31/ad971a3a-3430-4caa-9e7f-df60cc75872c__AMBARI-18281.v1.patch


Thanks,

Vivek Ratnavel Subramanian

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Jaimin Jetly]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/#review147460
---


Ship it!




Ship It!

- Jaimin Jetly


On Aug. 31, 2016, 1:51 a.m., Vivek Ratnavel Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51540/
> ---
> 
> (Updated Aug. 31, 2016, 1:51 a.m.)
> 
> 
> Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.
> 
> 
> Bugs: Ambari-18281
> https://issues.apache.org/jira/browse/Ambari-18281
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Add an option to Enable/ Disable alert notifications in Manage Notifications 
> popup.
> 
> 
> Diffs
> -
> 
>   
> ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js
>  1de15a6 
>   ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
>   ambari-web/app/messages.js b216918 
>   ambari-web/app/models/alerts/alert_notification.js c2d7570 
>   ambari-web/app/styles/modal_popups.less 762ea46 
>   ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
> 19f9ca3 
>   ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 
>   ambari-web/test/views/main/alerts/manage_alert_notifications_view_test.js 
> cebee96 
> 
> Diff: https://reviews.apache.org/r/51540/diff/
> 
> 
> Testing
> ---
> 
> Verified manually.
> Ambari-web unit tests pass.
> 30014 tests complete (26 seconds)
> 154 tests pending
> 
> 
> File Attachments
> 
> 
> v1
>   
> https://reviews.apache.org/media/uploaded/files/2016/08/31/ad971a3a-3430-4caa-9e7f-df60cc75872c__AMBARI-18281.v1.patch
> 
> 
> Thanks,
> 
> Vivek Ratnavel Subramanian
> 
>

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Vivek Ratnavel Subramanian]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/
---

(Updated Aug. 31, 2016, 1:51 a.m.)


Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.


Changes
---

Modified unit test.
Fixed a bug with tooltips.
Ambari-web unit tests pass.
30014 tests complete (27 seconds)
154 tests pending


Bugs: Ambari-18281
https://issues.apache.org/jira/browse/Ambari-18281


Repository: ambari


Description
---

Add an option to Enable/ Disable alert notifications in Manage Notifications 
popup.


Diffs (updated)
-

  
ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js 
1de15a6 
  ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
  ambari-web/app/messages.js b216918 
  ambari-web/app/models/alerts/alert_notification.js c2d7570 
  ambari-web/app/styles/modal_popups.less 762ea46 
  ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
19f9ca3 
  ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 
  ambari-web/test/views/main/alerts/manage_alert_notifications_view_test.js 
cebee96 

Diff: https://reviews.apache.org/r/51540/diff/


Testing
---

Verified manually.
Ambari-web unit tests pass.
30014 tests complete (26 seconds)
154 tests pending


File Attachments


v1
  
https://reviews.apache.org/media/uploaded/files/2016/08/31/ad971a3a-3430-4caa-9e7f-df60cc75872c__AMBARI-18281.v1.patch


Thanks,

Vivek Ratnavel Subramanian

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Vivek Ratnavel Subramanian]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/
---

(Updated Aug. 31, 2016, 1:48 a.m.)


Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.


Changes
---

Modified unit test.
Fixed a bug with tooltips.
Ambari-web unit tests pass.
30014 tests complete (27 seconds)
154 tests pending


Bugs: Ambari-18281
https://issues.apache.org/jira/browse/Ambari-18281


Repository: ambari


Description
---

Add an option to Enable/ Disable alert notifications in Manage Notifications 
popup.


Diffs
-

  
ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js 
1de15a6 
  ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
  ambari-web/app/messages.js b216918 
  ambari-web/app/models/alerts/alert_notification.js c2d7570 
  ambari-web/app/styles/modal_popups.less 762ea46 
  ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
19f9ca3 
  ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 

Diff: https://reviews.apache.org/r/51540/diff/


Testing
---

Verified manually.
Ambari-web unit tests pass.
30014 tests complete (26 seconds)
154 tests pending


File Attachments (updated)


v1
  
https://reviews.apache.org/media/uploaded/files/2016/08/31/ad971a3a-3430-4caa-9e7f-df60cc75872c__AMBARI-18281.v1.patch


Thanks,

Vivek Ratnavel Subramanian

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Vivek Ratnavel Subramanian]

> On Aug. 30, 2016, 9:43 p.m., Jaimin Jetly wrote:
> > ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs, 
> > lines 47-56
> > 
> >
> > Is hidding this section of code when "selectedAlertNotificaion.enabled 
> > is true" intentional ?

Yes, the Edit and Duplicate options should be visible only when the alert 
notification is enabled. If it is disabled, then only "Enable" option is 
visible.


- Vivek Ratnavel


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/#review147365
---


On Aug. 30, 2016, 9:24 p.m., Vivek Ratnavel Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51540/
> ---
> 
> (Updated Aug. 30, 2016, 9:24 p.m.)
> 
> 
> Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.
> 
> 
> Bugs: Ambari-18281
> https://issues.apache.org/jira/browse/Ambari-18281
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Add an option to Enable/ Disable alert notifications in Manage Notifications 
> popup.
> 
> 
> Diffs
> -
> 
>   
> ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js
>  1de15a6 
>   ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
>   ambari-web/app/messages.js b216918 
>   ambari-web/app/models/alerts/alert_notification.js c2d7570 
>   ambari-web/app/styles/modal_popups.less 762ea46 
>   ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
> 19f9ca3 
>   ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 
> 
> Diff: https://reviews.apache.org/r/51540/diff/
> 
> 
> Testing
> ---
> 
> Verified manually.
> Ambari-web unit tests pass.
> 30014 tests complete (26 seconds)
> 154 tests pending
> 
> 
> Thanks,
> 
> Vivek Ratnavel Subramanian
> 
>

Re: Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Jaimin Jetly]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/#review147365
---




ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js 
(lines 886 - 901)


disableAlertNotification and EnableAlertNotification seems to be similar in 
code except value of enabled. Lets make it an argument and then have just one 
function.



ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js 
(line 907)


enableAlertNotificationSuccessCallback and 
disableAlertNotificationSuccessCallback seems to be exactly same. This can also 
be the same function being called from same parent function as suggested on 
above comment.



ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
(lines 47 - 56)


Is hidding this section of code when "selectedAlertNotificaion.enabled is 
true" intentional ?


- Jaimin Jetly


On Aug. 30, 2016, 9:24 p.m., Vivek Ratnavel Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51540/
> ---
> 
> (Updated Aug. 30, 2016, 9:24 p.m.)
> 
> 
> Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.
> 
> 
> Bugs: Ambari-18281
> https://issues.apache.org/jira/browse/Ambari-18281
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Add an option to Enable/ Disable alert notifications in Manage Notifications 
> popup.
> 
> 
> Diffs
> -
> 
>   
> ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js
>  1de15a6 
>   ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
>   ambari-web/app/messages.js b216918 
>   ambari-web/app/models/alerts/alert_notification.js c2d7570 
>   ambari-web/app/styles/modal_popups.less 762ea46 
>   ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
> 19f9ca3 
>   ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 
> 
> Diff: https://reviews.apache.org/r/51540/diff/
> 
> 
> Testing
> ---
> 
> Verified manually.
> Ambari-web unit tests pass.
> 30014 tests complete (26 seconds)
> 154 tests pending
> 
> 
> Thanks,
> 
> Vivek Ratnavel Subramanian
> 
>

Review Request 51540: Expose Disabling of Alert Targets in Web Client

[Vivek Ratnavel Subramanian]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51540/
---

Review request for Ambari, Jaimin Jetly, Zhe (Joe) Wang, and Yusaku Sako.


Bugs: Ambari-18281
https://issues.apache.org/jira/browse/Ambari-18281


Repository: ambari


Description
---

Add an option to Enable/ Disable alert notifications in Manage Notifications 
popup.


Diffs
-

  
ambari-web/app/controllers/main/alerts/manage_alert_notifications_controller.js 
1de15a6 
  ambari-web/app/mappers/alert_notification_mapper.js 95e2362 
  ambari-web/app/messages.js b216918 
  ambari-web/app/models/alerts/alert_notification.js c2d7570 
  ambari-web/app/styles/modal_popups.less 762ea46 
  ambari-web/app/templates/main/alerts/manage_alert_notifications_popup.hbs 
19f9ca3 
  ambari-web/app/views/main/alerts/manage_alert_notifications_view.js 5b476a3 

Diff: https://reviews.apache.org/r/51540/diff/


Testing
---

Verified manually.
Ambari-web unit tests pass.
30014 tests complete (26 seconds)
154 tests pending


Thanks,

Vivek Ratnavel Subramanian