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

BZ#1637512-Restore action cable subscription, ensure enablement flag is available #1482

Merged
merged 1 commit into from
Oct 12, 2018

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Oct 9, 2018

fixes https://bugzilla.redhat.com/show_bug.cgi?id=1637512

Pretty much reimplements #1314, but now its working without errors

what it looks like, but yah really should jump in for yerself

test

and if you jump in... @skateman provided this wonderfully useful commands... (to be run from rails console)

Notification.create(:type => :generic_task_fail, :subject => Vm.first, :options => {:message => "nil"}, :user_id => User.first.id)

or

Notification.last.send(:emit_message)

@skateman
Copy link
Member

@AllenBW the ActionCable is initialized in one more place:

$ ag cable.subscriptions.create
client/app/states/login/login.state.js
79:      cable.subscriptions.create('NotificationChannel', {})

client/app/core/authorization.config.js
96:      cable.subscriptions.create('NotificationChannel', {})

@AllenBW AllenBW force-pushed the bug/#1637512-broken-websockets branch from ff34862 to 879ee5b Compare October 10, 2018 10:37
@AllenBW
Copy link
Member Author

AllenBW commented Oct 10, 2018

@skateman good catch, doesn't change behavior in above gif, updated!

@skateman
Copy link
Member

@AllenBW okay, but what if you force the refresh of a page (F5)?

@AllenBW
Copy link
Member Author

AllenBW commented Oct 10, 2018

@skateman its all ok when you force refresh of a page (F5). I did that, then sent a notification, it showed up.

@skateman
Copy link
Member

but travis doesn't 😞

@himdel
Copy link
Contributor

himdel commented Oct 10, 2018

10 10 2018 11:45:13.336:WARN [PhantomJS 2.1.1 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  Disconnected, because no message in 10000 ms.
PhantomJS 2.1.1 (Linux 0.0.0): Executed 6 of 357 DISCONNECTED (11.739 secs / 1.163 secs)
PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  Disconnected, because no message in 10000 ms.
JS 2.1.1 (Linux 0.0.0): Executed 6 of 357 DISCONNECTED (11.739 secs / 1.163 secs)

hopefully just a restart..

@@ -23,7 +23,8 @@ export function ApplianceInfo ($sessionStorage) {
role: data.identity.role,
suiVersion: gitHash.gitCommit,
miqVersion: data.server_info.version + '.' + data.server_info.build,
server: data.server_info.appliance
server: data.server_info.appliance,
asyncNotify: data.settings.asynchronous_notifications || false
Copy link
Member

Choose a reason for hiding this comment

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

This should be true by default, because the feature behind this is to turn it OFF.

Copy link
Member Author

Choose a reason for hiding this comment

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

good info!

@@ -40,7 +40,8 @@ describe('Appliance Info Service', () => {
'role': 'EvmRole-super_administrator',
'miqVersion': 'master.20170510164252_9e5df30',
'suiVersion': '',
'server': 'EVM'
'server': 'EVM',
'asyncNotify': 'false'
Copy link
Member

Choose a reason for hiding this comment

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

true please

Copy link
Member Author

Choose a reason for hiding this comment

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

fiiineeeeee

@AllenBW AllenBW force-pushed the bug/#1637512-broken-websockets branch from 879ee5b to a30d4b1 Compare October 10, 2018 12:11
@AllenBW
Copy link
Member Author

AllenBW commented Oct 10, 2018

Yah needs a restart, maybe a few, timeouts are killin' travis, not spec fails 😋

@AllenBW
Copy link
Member Author

AllenBW commented Oct 10, 2018

@himdel @skateman see TRAVIS IS A FRIEND NOT FOOD, just needs a lil lovin' 'sall

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

Copy link
Contributor

@himdel himdel left a comment

Choose a reason for hiding this comment

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

LGTM 👍

A bit worried that we're initializing this in a service constructor,
but verified this service doesn't get instantiated on the login page,
and it is used by menus and the shopping cart, so it will be initialized on every page.

@himdel himdel merged commit 3af8f1b into ManageIQ:master Oct 12, 2018
@himdel himdel added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 12, 2018
@AllenBW AllenBW deleted the bug/#1637512-broken-websockets branch October 12, 2018 14:19
@AllenBW
Copy link
Member Author

AllenBW commented Oct 12, 2018

<3

simaishi pushed a commit that referenced this pull request Oct 12, 2018
BZ#1637512-Restore action cable subscription, ensure enablement flag is available

(cherry picked from commit 3af8f1b)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1637512
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 0d9b99cffa379f24dc1ba547ba8e8f5f11766b0b
Author: Martin Hradil <[email protected]>
Date:   Fri Oct 12 15:47:04 2018 +0200

    Merge pull request #1482 from AllenBW/bug/#1637512-broken-websockets
    
    BZ#1637512-Restore action cable subscription, ensure enablement flag is available
    
    (cherry picked from commit 3af8f1b78e1fef72ba059ce8552fe048331de03c)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1637512

simaishi pushed a commit that referenced this pull request Nov 5, 2018
BZ#1637512-Restore action cable subscription, ensure enablement flag is available

(cherry picked from commit 3af8f1b)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1639351
@simaishi
Copy link
Contributor

simaishi commented Nov 5, 2018

Gaprindashvili backport details:

$ git log -1
commit ff4c2c219bbc4e093bdffdcdadeda38d2bddcdd0
Author: Martin Hradil <[email protected]>
Date:   Fri Oct 12 15:47:04 2018 +0200

    Merge pull request #1482 from AllenBW/bug/#1637512-broken-websockets
    
    BZ#1637512-Restore action cable subscription, ensure enablement flag is available
    
    (cherry picked from commit 3af8f1b78e1fef72ba059ce8552fe048331de03c)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1639351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants