From 5fd3a04790ecc6ea9b8a5c9d0a3bc9765fb83195 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 21 2017 13:40:40 +0000 Subject: [PATCH 1/35] Make the Feed component more reusable --- diff --git a/hubs/static/client/app/__tests__/Feed.test.js b/hubs/static/client/app/__tests__/Feed.test.js deleted file mode 100644 index 76daf32..0000000 --- a/hubs/static/client/app/__tests__/Feed.test.js +++ /dev/null @@ -1,34 +0,0 @@ -/* eslint-env jasmine, jest */ - -jest.unmock('../widgets/feed/Feed.jsx'); - -import React from 'react'; -import TestUtils from 'react-addons-test-utils'; - -import Feed from '../widgets/feed/Feed.jsx'; - -describe('Feed', () => { - const matches = [{ - markup: 'Link', - date_time: new Date(), - link: 'https://pagure.io/fedora-hubs', - secondary_icon: 'https://placekitten.com/g/200/300', - }]; - - const options = { - messageLimit: 100 - }; - - it('should set the correct state', () => { - const component = TestUtils.renderIntoDocument( - - ); - - const state = { - matches, - messageLimit: 100, - sse: true, - }; - expect(component.state).toEqual(state); - }); -}); diff --git a/hubs/static/client/app/__tests__/Icon.test.js b/hubs/static/client/app/__tests__/Icon.test.js deleted file mode 100644 index 920c853..0000000 --- a/hubs/static/client/app/__tests__/Icon.test.js +++ /dev/null @@ -1,36 +0,0 @@ -/* eslint-env jasmine, jest */ - -jest.unmock('../components/Icon.jsx'); - -import React from 'react'; -import ReactDOM from 'react-dom'; -import TestUtils from 'react-addons-test-utils'; - -import Icon from '../components/Icon.jsx'; - -describe('Icon', () => { - const match = { - link: 'https://pagure.io/fedora-hubs', - secondary_icon: 'https://placekitten.com/g/200/300', - }; - - it('has the correct link', () => { - const component = TestUtils.renderIntoDocument( - - ); - expect(component).toBeTruthy(); - const node = ReactDOM.findDOMNode(component); - const link = node.querySelector('a'); - expect(link.getAttribute('href')).toEqual(match.link); - }); - it('has the correct image source', () => { - const component = TestUtils.renderIntoDocument( - - ); - expect(component).toBeTruthy(); - - const node = ReactDOM.findDOMNode(component); - const image = node.querySelector('img'); - expect(image.getAttribute('src')).toEqual(match.secondary_icon); - }); -}); diff --git a/hubs/static/client/app/__tests__/Markup.test.js b/hubs/static/client/app/__tests__/Markup.test.js deleted file mode 100644 index d2aea84..0000000 --- a/hubs/static/client/app/__tests__/Markup.test.js +++ /dev/null @@ -1,30 +0,0 @@ -/* eslint-env jasmine, jest */ - -jest.unmock('../components/Markup.jsx'); - -import React from 'react'; -import ReactDOM from 'react-dom'; -import TestUtils from 'react-addons-test-utils'; - -import Markup from '../components/Markup.jsx'; - -describe('Markup', () => { - const match = { - markup: 'Link', - date_time: new Date(), - }; - - it('should set the inner html of the markup', () => { - const component = TestUtils.renderIntoDocument( - - ); - expect(component).toBeTruthy(); - - const node = ReactDOM.findDOMNode(component); - const header = node.querySelector('h4'); - - expect(header.innerHTML).toEqual(match.markup); - }); - - // we don't test TimeAgo, the library should have its own tests -}); diff --git a/hubs/static/client/app/__tests__/Panel.test.js b/hubs/static/client/app/__tests__/Panel.test.js deleted file mode 100644 index c8632c5..0000000 --- a/hubs/static/client/app/__tests__/Panel.test.js +++ /dev/null @@ -1,30 +0,0 @@ -/* eslint-env jasmine, jest */ - -jest.unmock('../components/Panel.jsx'); - -import React from 'react'; -import ReactDOM from 'react-dom'; -import TestUtils from 'react-addons-test-utils'; - -import Panel from '../components/Panel.jsx'; - -describe('Panel', () => { - const match = { - markup: 'Link', - date_time: new Date(), - link: 'https://pagure.io/fedora-hubs', - secondary_icon: 'https://placekitten.com/g/200/300', - }; - - it('should render the card block', () => { - const component = TestUtils.renderIntoDocument( - - ); - expect(component).toBeTruthy(); - - const node = ReactDOM.findDOMNode(component); - const block = node.querySelector('.card-block'); - - expect(block).toBeTruthy(); - }); -}); diff --git a/hubs/static/client/app/components/Dropdown.jsx b/hubs/static/client/app/components/Dropdown.jsx deleted file mode 100644 index efa6201..0000000 --- a/hubs/static/client/app/components/Dropdown.jsx +++ /dev/null @@ -1,71 +0,0 @@ -import React from 'react'; - -export default class Dropdown extends React.Component { - save() { - const payload = { - link: this.props.match.link, - markup: this.props.match.markup, - secondary_icon: this.props.match.secondary_icon, - dom_id: this.props.match.dom_id, - }; - - $.ajax({ - type: 'POST', - url: this.props.options.saveUrl, - data: JSON.stringify(payload), - contentType: 'application/json', - }).done(() => { - const id = `#save-${this.props.match.dom_id}`; - const $saveBtn = $(id); - $saveBtn.removeClass('btn-primary').addClass('btn-success'); - $saveBtn.text('Saved'); - }); - } - - delete() { - $.ajax({ - type: 'DELETE', - url: `${this.props.options.saveUrl}${this.props.match.idx}/`, - }).done((resp) => { - if (resp.status_code === 200) { - const $notification = $(`#${this.props.match.dom_id}`); - $notification.closest('.card-block').remove(); - } - }); - } - - render() { - let saveBtn; - if (!this.props.match.saved && this.props.options.saveUrl) { - saveBtn = ( - - ); - } - let deleteBtn; - if (this.props.options.delete && this.props.options.saveUrl) { - deleteBtn = ( - - ); - } - - return ( -
- {saveBtn} - {deleteBtn} -
- ); - } -} diff --git a/hubs/static/client/app/components/Icon.jsx b/hubs/static/client/app/components/Icon.jsx deleted file mode 100644 index 0eec3cc..0000000 --- a/hubs/static/client/app/components/Icon.jsx +++ /dev/null @@ -1,21 +0,0 @@ -import React from 'react'; - -class Icon extends React.Component { - render() { - return ( -
-
- - User avatar - -
-
- ); - } -} - -export default Icon; diff --git a/hubs/static/client/app/components/Markup.jsx b/hubs/static/client/app/components/Markup.jsx deleted file mode 100644 index 92c2c50..0000000 --- a/hubs/static/client/app/components/Markup.jsx +++ /dev/null @@ -1,28 +0,0 @@ -import React from 'react'; -import Dropdown from './Dropdown.jsx'; -import TimeAgo from 'react-timeago'; - - -export default class Markup extends React.Component { - createMarkup() { - return { __html: this.props.match.markup }; - } - render() { - const timestamp = this.props.match.date_time ? () : null; - - return ( -
-

-

- {timestamp} - -
- ); - } -} diff --git a/hubs/static/client/app/components/Panel.jsx b/hubs/static/client/app/components/Panel.jsx deleted file mode 100644 index 848a5b7..0000000 --- a/hubs/static/client/app/components/Panel.jsx +++ /dev/null @@ -1,19 +0,0 @@ -import React from 'react'; - -import Icon from './Icon.jsx'; -import Markup from './Markup.jsx'; - -class Panel extends React.Component { - render() { - return ( -
-
- - -
-
- ); - } -} - -export default Panel; diff --git a/hubs/static/client/app/components/feed/Actions.jsx b/hubs/static/client/app/components/feed/Actions.jsx new file mode 100644 index 0000000..c481377 --- /dev/null +++ b/hubs/static/client/app/components/feed/Actions.jsx @@ -0,0 +1,78 @@ +import React from 'react'; + + +export default class Actions extends React.Component { + + constructor(props) { + super(props); + this.save = this.save.bind(this); + this.delete = this.delete.bind(this); + } + + save() { + const payload = { + link: this.props.item.link, + markup: this.props.item.markup, + secondary_icon: this.props.item.secondary_icon, + dom_id: this.props.item.dom_id, + }; + + $.ajax({ + type: 'POST', + url: this.props.options.saveUrl, + data: JSON.stringify(payload), + contentType: 'application/json', + }).done(() => { + const id = `#save-${this.props.item.dom_id}`; + const $saveBtn = $(id); + $saveBtn.removeClass('btn-primary').addClass('btn-success'); + $saveBtn.text('Saved'); + }); + } + + delete() { + $.ajax({ + type: 'DELETE', + url: `${this.props.options.saveUrl}${this.props.item.idx}/`, + }).done((resp) => { + if (resp.status_code === 200) { + const $notification = $(`#${this.props.item.dom_id}`); + $notification.closest('.card-block').remove(); + } + }); + } + + render() { + var buttonProps = { + id: `save-${this.props.item.dom_id}`, + className: "btn btn-sm ", + }; + var buttonText; + + if (this.props.item.saved) { + if (this.props.options.delete) { + buttonProps.className += "btn-danger"; + buttonProps.onClick = this.delete; + buttonText = "Remove"; + } else { + buttonText = "Saved"; + } + } else { + buttonProps.className += "btn-primary"; + buttonProps.onClick = this.save; + buttonText = "Save"; + } + + return ( +
+ +
+ ); + } + +} + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/Feed.jsx b/hubs/static/client/app/components/feed/Feed.jsx new file mode 100644 index 0000000..464e2e1 --- /dev/null +++ b/hubs/static/client/app/components/feed/Feed.jsx @@ -0,0 +1,24 @@ +import React from 'react'; +import Panel from './Panel.jsx'; + + +export default class Feed extends React.Component { + + render() { + var items = this.props.items || []; + items = items.map((item, idx) => { + return ( + + ); + }); + return ( +
+ {items} +
+ ); + } + +} + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/Icon.jsx b/hubs/static/client/app/components/feed/Icon.jsx new file mode 100644 index 0000000..a81f41c --- /dev/null +++ b/hubs/static/client/app/components/feed/Icon.jsx @@ -0,0 +1,21 @@ +import React from 'react'; + +class Icon extends React.Component { + render() { + return ( +
+
+ + User avatar + +
+
+ ); + } +} + +export default Icon; diff --git a/hubs/static/client/app/components/feed/Markup.jsx b/hubs/static/client/app/components/feed/Markup.jsx new file mode 100644 index 0000000..daf2d84 --- /dev/null +++ b/hubs/static/client/app/components/feed/Markup.jsx @@ -0,0 +1,36 @@ +import React from 'react'; +import Actions from './Actions.jsx'; +import TimeAgo from 'react-timeago'; + + +export default class Markup extends React.Component { + + constructor(props) { + super(props); + this.createMarkup = this.createMarkup.bind(this); + } + + createMarkup() { + return { __html: this.props.item.markup }; + } + + render() { + const timestamp = this.props.item.date_time ? () : null; + + return ( +
+

+

+ {timestamp} + +
+ ); + } + +} + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/Panel.jsx b/hubs/static/client/app/components/feed/Panel.jsx new file mode 100644 index 0000000..eb0128b --- /dev/null +++ b/hubs/static/client/app/components/feed/Panel.jsx @@ -0,0 +1,22 @@ +import React from 'react'; +import Icon from './Icon.jsx'; +import Markup from './Markup.jsx'; + + +export default class Panel extends React.Component { + + render() { + return ( +
+
+ + +
+
+ ); + } + +} + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/__tests__/Feed.test.js b/hubs/static/client/app/components/feed/__tests__/Feed.test.js new file mode 100644 index 0000000..a3e8365 --- /dev/null +++ b/hubs/static/client/app/components/feed/__tests__/Feed.test.js @@ -0,0 +1,30 @@ +/* eslint-env jasmine, jest */ + +jest.unmock('../Feed.jsx'); + +import React from 'react'; +import ReactDOM from 'react-dom'; +import TestUtils from 'react-addons-test-utils'; + +import Feed from '../Feed.jsx'; + +describe('Feed', () => { + const item = { + markup: 'Link', + date_time: new Date(), + link: 'https://pagure.io/fedora-hubs', + secondary_icon: 'https://placekitten.com/g/200/300', + } + const items = [item, item, item]; + + it('should create the children', () => { + const component = TestUtils.renderIntoDocument( + + ); + const node = ReactDOM.findDOMNode(component); + expect(node.children.length).toEqual(3); + }); +}); + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/__tests__/Icon.test.js b/hubs/static/client/app/components/feed/__tests__/Icon.test.js new file mode 100644 index 0000000..72dadc5 --- /dev/null +++ b/hubs/static/client/app/components/feed/__tests__/Icon.test.js @@ -0,0 +1,39 @@ +/* eslint-env jasmine, jest */ + +jest.unmock('../Icon.jsx'); + +import React from 'react'; +import ReactDOM from 'react-dom'; +import TestUtils from 'react-addons-test-utils'; + +import Icon from '../Icon.jsx'; + +describe('Icon', () => { + const item = { + link: 'https://pagure.io/fedora-hubs', + secondary_icon: 'https://placekitten.com/g/200/300', + }; + + it('has the correct link', () => { + const component = TestUtils.renderIntoDocument( + + ); + expect(component).toBeTruthy(); + const node = ReactDOM.findDOMNode(component); + const link = node.querySelector('a'); + expect(link.getAttribute('href')).toEqual(item.link); + }); + it('has the correct image source', () => { + const component = TestUtils.renderIntoDocument( + + ); + expect(component).toBeTruthy(); + + const node = ReactDOM.findDOMNode(component); + const image = node.querySelector('img'); + expect(image.getAttribute('src')).toEqual(item.secondary_icon); + }); +}); + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/__tests__/Markup.test.js b/hubs/static/client/app/components/feed/__tests__/Markup.test.js new file mode 100644 index 0000000..31d336d --- /dev/null +++ b/hubs/static/client/app/components/feed/__tests__/Markup.test.js @@ -0,0 +1,33 @@ +/* eslint-env jasmine, jest */ + +jest.unmock('../Markup.jsx'); + +import React from 'react'; +import ReactDOM from 'react-dom'; +import TestUtils from 'react-addons-test-utils'; + +import Markup from '../Markup.jsx'; + +describe('Markup', () => { + const item = { + markup: 'Link', + date_time: new Date(), + }; + + it('should set the inner html of the markup', () => { + const component = TestUtils.renderIntoDocument( + + ); + expect(component).toBeTruthy(); + + const node = ReactDOM.findDOMNode(component); + const header = node.querySelector('h4'); + + expect(header.innerHTML).toEqual(item.markup); + }); + + // we don't test TimeAgo, the library should have its own tests +}); + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/__tests__/Panel.test.js b/hubs/static/client/app/components/feed/__tests__/Panel.test.js new file mode 100644 index 0000000..6e17d1c --- /dev/null +++ b/hubs/static/client/app/components/feed/__tests__/Panel.test.js @@ -0,0 +1,33 @@ +/* eslint-env jasmine, jest */ + +jest.unmock('../Panel.jsx'); + +import React from 'react'; +import ReactDOM from 'react-dom'; +import TestUtils from 'react-addons-test-utils'; + +import Panel from '../Panel.jsx'; + +describe('Panel', () => { + const item = { + markup: 'Link', + date_time: new Date(), + link: 'https://pagure.io/fedora-hubs', + secondary_icon: 'https://placekitten.com/g/200/300', + }; + + it('should render the card block', () => { + const component = TestUtils.renderIntoDocument( + + ); + expect(component).toBeTruthy(); + + const node = ReactDOM.findDOMNode(component); + const block = node.querySelector('.card-block'); + + expect(block).toBeTruthy(); + }); +}); + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/widgets/feed/Feed.js b/hubs/static/client/app/widgets/feed/Feed.js new file mode 100644 index 0000000..af0b1c6 --- /dev/null +++ b/hubs/static/client/app/widgets/feed/Feed.js @@ -0,0 +1,7 @@ +import Widget from './Widget.jsx'; + + +// Don't use the ES6 "export default" construct: +// http://stackoverflow.com/questions/40294870/module-exports-vs-export-default-in-node-js-and-es6 + +module.exports = {Widget}; diff --git a/hubs/static/client/app/widgets/feed/Feed.jsx b/hubs/static/client/app/widgets/feed/Feed.jsx deleted file mode 100644 index beec5f7..0000000 --- a/hubs/static/client/app/widgets/feed/Feed.jsx +++ /dev/null @@ -1,53 +0,0 @@ -import React from 'react'; -import Panel from '../../components/Panel.jsx'; - - -class Feed extends React.Component { - - constructor(props) { - super(props); - this.state = { - matches: this.props.matches, - messageLimit: this.props.options.messageLimit, - sse: true, - }; - // If it wasn't instantiated with a url or an error happened, bail - if (!this.props.url || !this.state.sse) { - return; - } - this.source = (!!window.EventSource) ? new EventSource(this.props.url) : null; - if (!this.source) { - return; - } - this.source.addEventListener('error', () => { - this.state.sse = false; - }, false); - window.onbeforeunload = () => { - this.source.close(); - }; - this.source.onmessage = resp => { - const data = JSON.parse(resp.data); - if (this.state.matches.length >= this.state.messageLimit) { - this.state.matches.pop(); - } - this.state.matches.unshift(data); - this.setState({ matches: this.state.matches }); - }; - } - - render() { - const feedNodes = this.state.matches.map((match, idx) => { - return ; - }); - return ( -
- {feedNodes} -
- ); - } - -} - -// Don't use the ES6 "export default" construct: -// http://stackoverflow.com/questions/40294870/module-exports-vs-export-default-in-node-js-and-es6 -module.exports = Feed diff --git a/hubs/static/client/app/widgets/feed/Widget.jsx b/hubs/static/client/app/widgets/feed/Widget.jsx new file mode 100644 index 0000000..5ee232f --- /dev/null +++ b/hubs/static/client/app/widgets/feed/Widget.jsx @@ -0,0 +1,57 @@ +import React from 'react'; +import Feed from '../../components/feed/Feed.jsx'; + + +export default class Widget extends React.Component { + + constructor(props) { + super(props); + this.state = { + items: this.props.items, + sse: true, + }; + this.componentDidMount = this.componentDidMount.bind(this); + this.componentWillUnmount = this.componentWillUnmount.bind(this); + this.handleMessage = this.handleMessage.bind(this); + this.source = null; + } + + componentDidMount() { + // If it wasn't instantiated with a url or an error happened, bail + if (!this.props.url || !this.state.sse) { + return; + } + this.source = (!!window.EventSource) ? new EventSource(this.props.url) : null; + if (!this.source) { + return; + } + this.source.addEventListener('error', () => { + this.state.sse = false; + }, false); + this.source.onmessage = resp => { + this.handleMessage(JSON.parse(resp.data)); + }; + } + + componentWillUnmount() { + if (this.source) { this.source.close(); } + } + + handleMessage(msg) { + if (this.state.items.length >= this.props.options.messageLimit) { + this.state.items.pop(); + } + this.state.items.unshift(msg); + this.setState({items: this.state.items}); + } + + render() { + return ( + + ); + } + +} + + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/webpack.config.js b/hubs/static/client/webpack.config.js index f197a0e..532e978 100644 --- a/hubs/static/client/webpack.config.js +++ b/hubs/static/client/webpack.config.js @@ -9,7 +9,7 @@ const PATHS = { const config = { entry: { Hubs: path.join(PATHS.app, 'core', 'Hubs.js'), - Feed: path.join(PATHS.app, 'widgets', 'feed', 'Feed.jsx'), + Feed: path.join(PATHS.app, 'widgets', 'feed', 'Feed.js'), Halp: path.join(PATHS.app, 'widgets', 'halp', 'Halp.js') }, output: { diff --git a/hubs/templates/stream.html b/hubs/templates/stream.html index 7cfda81..b70b23d 100644 --- a/hubs/templates/stream.html +++ b/hubs/templates/stream.html @@ -67,8 +67,8 @@ src="{{url_for('static', filename='js/build/Feed.js')}}"> + src ="{{ url_for('static', filename='js/build/Feed.js') }}"> {% endblock %} diff --git a/hubs/views/user.py b/hubs/views/user.py index 0397b69..0a15cac 100644 --- a/hubs/views/user.py +++ b/hubs/views/user.py @@ -33,15 +33,27 @@ def stream(name): ) -@app.route('//notifications', methods=['GET', 'POST']) -@app.route('//notifications/', methods=['GET', 'POST']) +@app.route('//stream/existing') +@require_hub_access("view") +def stream_existing(name): + hub = get_hub(name) + stream = hubs.stream.Stream() + existing = json.loads(stream.get_json()) + saved = [s.dom_id for s in + hubs.models.SavedNotification.by_username(name)] + for e in existing: + e["saved"] = (e["dom_id"] in saved) + # Right now, stream and actions are the same. + # Once mentions is implemented, then each will be its own. + return flask.jsonify(dict( + status="OK", data=dict(notif=existing, action=existing), + )) + + +@app.route('//notifications', methods=['POST']) +@app.route('//notifications/', methods=['POST']) @login_required def notifications(user): - if flask.request.method == 'GET': - notifications = hubs.models.SavedNotification.by_username(user) - notifications = [n.__json__() for n in notifications] - return flask.jsonify(dict(notifications=notifications)) - if flask.request.method == 'POST': data = flask.request.get_json() user = hubs.models.User.by_username(user) @@ -63,9 +75,9 @@ def notifications(user): ) flask.g.db.add(notification) flask.g.db.commit() - return flask.jsonify( - dict(notification=notification.__json__(), success=True) - ) + return flask.jsonify(dict( + status="OK", data=notification.__json__(), + )) @app.route('//notifications/', methods=['DELETE']) @@ -78,7 +90,7 @@ def delete_notifications(user, idx): return flask.abort(400) flask.g.db.delete(notification) flask.g.db.commit() - return flask.jsonify(dict(status_code=200)) + return flask.jsonify(dict(status="OK")) @app.route('/visit//', methods=['GET', 'POST']) From 7138b0b0a6e5e30c4519772efe5ee798eb95b663 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 21 2017 13:40:40 +0000 Subject: [PATCH 4/35] Improve display of the Feed widget --- diff --git a/hubs/static/client/app/components/feed/Feed.jsx b/hubs/static/client/app/components/feed/Feed.jsx index 464e2e1..9292add 100644 --- a/hubs/static/client/app/components/feed/Feed.jsx +++ b/hubs/static/client/app/components/feed/Feed.jsx @@ -8,11 +8,11 @@ export default class Feed extends React.Component { var items = this.props.items || []; items = items.map((item, idx) => { return ( - + ); }); return ( -
+
{items}
); diff --git a/hubs/static/client/app/components/feed/Icon.jsx b/hubs/static/client/app/components/feed/Icon.jsx index a81f41c..3a15c77 100644 --- a/hubs/static/client/app/components/feed/Icon.jsx +++ b/hubs/static/client/app/components/feed/Icon.jsx @@ -1,21 +1,21 @@ import React from 'react'; -class Icon extends React.Component { +export default class Icon extends React.Component { render() { return ( -
-
- - User avatar - -
-
+ + User avatar + ); } } -export default Icon; + +// vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/components/feed/ItemsGetter.jsx b/hubs/static/client/app/components/feed/ItemsGetter.jsx index 1ba1c4f..16d87de 100644 --- a/hubs/static/client/app/components/feed/ItemsGetter.jsx +++ b/hubs/static/client/app/components/feed/ItemsGetter.jsx @@ -90,7 +90,7 @@ export default class ItemsGetter extends React.Component { style={{display: this.state.loading ? "block" : "none"}} >
{ (this.state.sseError !== false) && -
+
{this.state.sseError}
} diff --git a/hubs/static/client/app/components/feed/Markup.jsx b/hubs/static/client/app/components/feed/Markup.jsx index 72f88c9..16658eb 100644 --- a/hubs/static/client/app/components/feed/Markup.jsx +++ b/hubs/static/client/app/components/feed/Markup.jsx @@ -1,5 +1,4 @@ import React from 'react'; -import Actions from './Actions.jsx'; import TimeAgo from 'react-timeago'; @@ -18,16 +17,9 @@ export default class Markup extends React.Component { const timestamp = this.props.item.date_time ? () : null; return ( -
-

-

+
+ {timestamp} - { (this.props.handleSave) && - - }
); } diff --git a/hubs/static/client/app/components/feed/Panel.jsx b/hubs/static/client/app/components/feed/Panel.jsx index eb0128b..af5f440 100644 --- a/hubs/static/client/app/components/feed/Panel.jsx +++ b/hubs/static/client/app/components/feed/Panel.jsx @@ -1,17 +1,19 @@ import React from 'react'; import Icon from './Icon.jsx'; import Markup from './Markup.jsx'; +import Actions from './Actions.jsx'; export default class Panel extends React.Component { render() { return ( -
-
- - -
+
+ + + { (this.props.handleSave) && + + }
); } diff --git a/hubs/static/client/app/components/feed/__tests__/Feed.test.js b/hubs/static/client/app/components/feed/__tests__/Feed.test.js index a3e8365..d1cdf24 100644 --- a/hubs/static/client/app/components/feed/__tests__/Feed.test.js +++ b/hubs/static/client/app/components/feed/__tests__/Feed.test.js @@ -7,6 +7,7 @@ import ReactDOM from 'react-dom'; import TestUtils from 'react-addons-test-utils'; import Feed from '../Feed.jsx'; +import Panel from '../Panel.jsx'; describe('Feed', () => { const item = { @@ -15,14 +16,17 @@ describe('Feed', () => { link: 'https://pagure.io/fedora-hubs', secondary_icon: 'https://placekitten.com/g/200/300', } - const items = [item, item, item]; + var items = [item, item, item]; + items = items.map((obj, idx) => { + return Object.assign({dom_id: "item-" + idx}, obj); + }); it('should create the children', () => { const component = TestUtils.renderIntoDocument( ); const node = ReactDOM.findDOMNode(component); - expect(node.children.length).toEqual(3); + expect(Panel.mock.calls.length).toEqual(3); }); }); diff --git a/hubs/static/client/app/components/feed/__tests__/Icon.test.js b/hubs/static/client/app/components/feed/__tests__/Icon.test.js index 72dadc5..3cd5bb8 100644 --- a/hubs/static/client/app/components/feed/__tests__/Icon.test.js +++ b/hubs/static/client/app/components/feed/__tests__/Icon.test.js @@ -20,8 +20,7 @@ describe('Icon', () => { ); expect(component).toBeTruthy(); const node = ReactDOM.findDOMNode(component); - const link = node.querySelector('a'); - expect(link.getAttribute('href')).toEqual(item.link); + expect(node.getAttribute('href')).toEqual(item.link); }); it('has the correct image source', () => { const component = TestUtils.renderIntoDocument( diff --git a/hubs/static/client/app/components/feed/__tests__/Markup.test.js b/hubs/static/client/app/components/feed/__tests__/Markup.test.js index 31d336d..6305d51 100644 --- a/hubs/static/client/app/components/feed/__tests__/Markup.test.js +++ b/hubs/static/client/app/components/feed/__tests__/Markup.test.js @@ -21,9 +21,9 @@ describe('Markup', () => { expect(component).toBeTruthy(); const node = ReactDOM.findDOMNode(component); - const header = node.querySelector('h4'); + const markup = node.querySelector('span'); - expect(header.innerHTML).toEqual(item.markup); + expect(markup.innerHTML).toEqual(item.markup); }); // we don't test TimeAgo, the library should have its own tests diff --git a/hubs/static/css/style.css b/hubs/static/css/style.css index 2b722b4..60880bc 100644 --- a/hubs/static/css/style.css +++ b/hubs/static/css/style.css @@ -547,6 +547,17 @@ font-size: 32pt; } +/* + * Widget Feed + */ + +.component-feed .media { + border-bottom: 1px solid rgba(0, 0, 0, .125); +} +.component-feed .media:last-child { + border: 0 +} + /** fedora bootstrap overrides **/ diff --git a/hubs/widgets/feed/templates/feed.html b/hubs/widgets/feed/templates/feed.html index 6f74ccb..87fbafa 100644 --- a/hubs/widgets/feed/templates/feed.html +++ b/hubs/widgets/feed/templates/feed.html @@ -1,7 +1,7 @@ {% extends "panel.html" %} {% block content %} -
+
diff --git a/hubs/tests/hubs_test.cfg b/hubs/tests/hubs_test.cfg index a910e72..bae6178 100644 --- a/hubs/tests/hubs_test.cfg +++ b/hubs/tests/hubs_test.cfg @@ -1,7 +1,8 @@ ### Secret key for the Flask application -SECRET_KEY='' +SECRET_KEY = '' ### url to the database server: + import os DB_URL = 'sqlite:///%s/test.db' % (os.path.dirname(os.path.abspath(__file__))) #DB_URL='sqlite:////tmp/fedocal_dev.sqlite' @@ -11,3 +12,8 @@ OIDC_CLIENT_SECRETS = os.path.join(os.path.dirname( os.path.abspath(__file__)), 'client_secrets.json') SITE_ADMINS = ["admin"] + +SSE_URL = { + "port": "8080", + "path": "/sse", +} diff --git a/hubs/tests/test_view_utils.py b/hubs/tests/test_view_utils.py index 6a6d116..ca49dac 100644 --- a/hubs/tests/test_view_utils.py +++ b/hubs/tests/test_view_utils.py @@ -89,28 +89,43 @@ class ViewUtilsTest(APPTest): def test_sse_url(self): with app.test_request_context(): - self.assertEqual(get_sse_url(), "http://localhost:8080/sse") + self.assertEqual(get_sse_url(""), "http://localhost:8080/sse/") + + with app.test_request_context(): + self.assertEqual( + get_sse_url("hub/ralph"), + "http://localhost:8080/sse/hub/ralph") with app.test_request_context(base_url='http://hubs.example.com/'): - self.assertEqual(get_sse_url(), "http://hubs.example.com:8080/sse") + self.assertEqual( + get_sse_url("hub/ralph"), + "http://hubs.example.com:8080/sse/hub/ralph") with app.test_request_context(base_url='http://example.com/hubs'): self.assertEqual(flask.url_for("index"), "/hubs/") # test validity - self.assertEqual(get_sse_url(), "http://example.com:8080/sse") + self.assertEqual( + get_sse_url("hub/ralph"), + "http://example.com:8080/sse/hub/ralph") with app.test_request_context(base_url='http://hubs.example.com/'): with app_config(app, { "SSE_URL": {"host": "hubs-sse.example.com"} }): - self.assertEqual(get_sse_url(), "http://hubs-sse.example.com/") + self.assertEqual( + get_sse_url("hub/ralph"), + "http://hubs-sse.example.com/hub/ralph") with app.test_request_context(base_url='http://hubs.example.com/'): with app_config(app, { "SSE_URL": {"host": "hubs-sse.example.com", "port": 8080} }): - self.assertEqual(get_sse_url(), "http://hubs-sse.example.com:8080/") + self.assertEqual( + get_sse_url("hub/ralph"), + "http://hubs-sse.example.com:8080/hub/ralph") with app.test_request_context(base_url='http://localhost/hubs'): with app_config(app, {"SSE_URL": {"host": "example.com"}}): - self.assertEqual(get_sse_url(), "http://example.com/") + self.assertEqual( + get_sse_url("hub/ralph"), + "http://example.com/hub/ralph") diff --git a/hubs/utils/views.py b/hubs/utils/views.py index 4a4c63a..50c5723 100644 --- a/hubs/utils/views.py +++ b/hubs/utils/views.py @@ -288,7 +288,7 @@ class RequestValidator(object): return values -def get_sse_url(): +def get_sse_url(target): """Build the SSE URL.""" # Avoid circular import with widgets. from hubs.app import app @@ -302,7 +302,11 @@ def get_sse_url(): netloc = host if port: netloc = "{}:{}".format(host, port) - path = urlparse.urljoin(base_url.path, conf.get("path", "/")) + path = conf.get("path", "") + if not path.endswith("/"): + path += "/" + path = urlparse.urljoin(base_url.path, path) + path = urlparse.urljoin(path, target) return urlparse.urlunsplit([ conf.get("scheme") or base_url.scheme, netloc, path, "", "", diff --git a/hubs/views/hub.py b/hubs/views/hub.py index 5496503..0616f05 100644 --- a/hubs/views/hub.py +++ b/hubs/views/hub.py @@ -22,7 +22,7 @@ def hub(name): hub=hub, widgets=widgets, edit=False, - sse_url=get_sse_url(), + sse_url=get_sse_url("hub/{}".format(hub.name)), ) diff --git a/hubs/views/user.py b/hubs/views/user.py index a5cc00d..13e1313 100644 --- a/hubs/views/user.py +++ b/hubs/views/user.py @@ -1,15 +1,11 @@ from __future__ import unicode_literals, absolute_import import flask -import json import hubs.models import hubs.feed from hubs.app import app -from hubs.utils.views import ( - login_required, get_hub, get_sse_url, get_visible_widgets, - require_hub_access, - ) +from hubs.utils.views import login_required, get_hub, get_sse_url @app.route('/stream') @@ -18,12 +14,10 @@ from hubs.utils.views import ( def stream(): username = flask.g.auth.user.username hub = get_hub(username) - widgets = get_visible_widgets(hub) return flask.render_template( 'stream.html', hub=hub, - widgets=widgets, - sse_url=get_sse_url(), + sse_url=get_sse_url("user/{}".format(username)), ) diff --git a/hubs/widgets/feed/__init__.py b/hubs/widgets/feed/__init__.py index eb0db3c..9010ad1 100644 --- a/hubs/widgets/feed/__init__.py +++ b/hubs/widgets/feed/__init__.py @@ -3,8 +3,6 @@ from __future__ import unicode_literals, absolute_import import logging -import hubs.feed -from hubs.views.utils import get_sse_url from hubs.widgets import validators from hubs.widgets.base import Widget, WidgetView @@ -39,7 +37,6 @@ class BaseView(WidgetView): def get_context(self, instance, *args, **kwargs): return dict( title=self.widget.label, - sse_url=get_sse_url(), disable_autoreload=True, ) diff --git a/requirements.txt b/requirements.txt index ed9742f..07dab88 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,3 +26,4 @@ pygments pygments-markdown-lexer redis retask +txredisapi diff --git a/systemd/hubs-sse.service b/systemd/hubs-sse.service new file mode 100644 index 0000000..1120a4f --- /dev/null +++ b/systemd/hubs-sse.service @@ -0,0 +1,19 @@ +[Unit] +Description=fedora-hubs SSE server +After=network.target +Documentation=https://pagure.io/fedora-hubs/ + +[Service] +ExecStart= \ + twistd -l - --pidfile= \ + -ny /srv/hubs/fedora-hubs/hubs/backend/sse_server.tac +WorkingDirectory=/srv/hubs/fedora-hubs/ +PIDFile=/run/hubs-sse.pid +Type=simple +User=root +Group=root +Restart=on-failure + +[Install] +WantedBy=multi-user.target + From 4355b079430255ac1d15cc9d8a548e5bc0607bdc Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 17/35] Add switches to the backend commands for debug output --- diff --git a/hubs/backend/triage.py b/hubs/backend/triage.py index b4a2c82..f59769b 100755 --- a/hubs/backend/triage.py +++ b/hubs/backend/triage.py @@ -21,6 +21,7 @@ handle. from __future__ import unicode_literals +import argparse import json import logging import logging.config @@ -98,10 +99,20 @@ def get_widgets(): return widgets +def parse_args(args): + parser = argparse.ArgumentParser( + description='Triage messages from the bus.') + parser.add_argument("-d", "--debug", action="store_true", + help="debugging output level.") + return parser.parse_args() + + def main(args=None): args = args if args is not None else sys.argv + args = parse_args(args) logging.config.dictConfig(fedmsg_config['logging']) - logging.basicConfig() + log_level = logging.DEBUG if args.debug else logging.INFO + logging.basicConfig(level=log_level) # XXX - for flask.url_for to work hubs.app.app.config['SERVER_NAME'] = '0.0.0.0:5000' @@ -122,7 +133,7 @@ def main(args=None): while True: task = inbound.wait() # Wait forever... timeout is optional. msg = json.loads(task.data) - log.info( + log.debug( "(triage backlog: %r, work backlog: %r) Working on %r %r", inbound.length, outbound.length, msg['msg_id'], msg['topic'], ) diff --git a/hubs/backend/worker.py b/hubs/backend/worker.py index 363025a..c3c2f35 100755 --- a/hubs/backend/worker.py +++ b/hubs/backend/worker.py @@ -26,6 +26,7 @@ clients with the new content produced here. from __future__ import unicode_literals +import argparse import json import logging import logging.config @@ -66,10 +67,19 @@ def handle_widget_cache(widget_idx, fn_name): session.close() +def parse_args(args): + parser = argparse.ArgumentParser(description='Rebuild widget caches.') + parser.add_argument("-d", "--debug", action="store_true", + help="debugging output level.") + return parser.parse_args() + + def main(args=None): args = args if args is not None else sys.argv + args = parse_args(args) logging.config.dictConfig(fedmsg_config['logging']) - logging.basicConfig() + log_level = logging.DEBUG if args.debug else logging.INFO + logging.basicConfig(level=log_level) # XXX - for flask.url_for to work hubs.app.app.config['SERVER_NAME'] = '0.0.0.0:5000' From 8ac9cae9c98be453a465da713233c77cfe9a89bc Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 18/35] Feed: add a toggle to show the events of an aggregate --- diff --git a/hubs/static/client/app/components/feed/Icon.jsx b/hubs/static/client/app/components/feed/Icon.jsx index 3a15c77..9e0cfbe 100644 --- a/hubs/static/client/app/components/feed/Icon.jsx +++ b/hubs/static/client/app/components/feed/Icon.jsx @@ -5,7 +5,7 @@ export default class Icon extends React.Component { return ( User avatar 1) { + submessages = Object.keys(submsgs).map((msgid) => { + return ( +
  • + +
    • + ); + }); + } return ( -
    - - - - +
    + + {timestamp && + + + + } + {submessages.length !== 0 && +
      + {submessages} +
    + }
    ); } diff --git a/hubs/static/client/app/components/feed/Panel.jsx b/hubs/static/client/app/components/feed/Panel.jsx index 80bed1c..5be00e7 100644 --- a/hubs/static/client/app/components/feed/Panel.jsx +++ b/hubs/static/client/app/components/feed/Panel.jsx @@ -6,14 +6,42 @@ import Actions from './Actions.jsx'; export default class Panel extends React.Component { + constructor(props) { + super(props); + this.state = { + detailsOpened: false, + } + this.toggleDetails = this.toggleDetails.bind(this); + } + + toggleDetails(e) { + e.preventDefault(); + this.setState((prevState, props) => ( + {detailsOpened: !prevState.detailsOpened} + )); + } + render() { return (     ); } diff --git a/hubs/static/client/app/components/feed/__tests__/Panel.test.js b/hubs/static/client/app/components/feed/__tests__/Panel.test.js index 6e17d1c..ff93dbe 100644 --- a/hubs/static/client/app/components/feed/__tests__/Panel.test.js +++ b/hubs/static/client/app/components/feed/__tests__/Panel.test.js @@ -14,19 +14,35 @@ describe('Panel', () => { date_time: new Date(), link: 'https://pagure.io/fedora-hubs', secondary_icon: 'https://placekitten.com/g/200/300', + msg_ids: { + foobar1: {}, + foobar2: {}, + }, }; - it('should render the card block', () => { + it('should render the actions block', () => { const component = TestUtils.renderIntoDocument( ); expect(component).toBeTruthy(); const node = ReactDOM.findDOMNode(component); - const block = node.querySelector('.card-block'); + const block = node.querySelector('div.ml-3.text-right'); expect(block).toBeTruthy(); }); + + it('should show the details link', () => { + const component = TestUtils.renderIntoDocument( + + ); + expect(component).toBeTruthy(); + + const node = ReactDOM.findDOMNode(component); + const details = node.querySelector('.text-right small a'); + + expect(details).toBeTruthy(); + }); }); From dcfbfb4ac366ae22036fdc791cd02fa3dc4a48f1 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 19/35] Create links for user hubs in Feed messages --- diff --git a/hubs/feed.py b/hubs/feed.py index 786844b..5fb9546 100644 --- a/hubs/feed.py +++ b/hubs/feed.py @@ -3,8 +3,10 @@ from __future__ import unicode_literals import hashlib import logging import os +import re import fedmsg.meta +import flask import redis from fedmsg.encoding import loads, dumps @@ -66,14 +68,41 @@ def format_msg(msg): # invariant through conglomerate() calls. msg["dom_id"] = hashlib.sha1( b":".join( - [mid.encode("utf-8") for mid in msg["msg_ids"]] + [mid.encode("utf-8") for mid in sorted(msg["msg_ids"])] )).hexdigest() # TODO: generate markup - msg["markup"] = msg["subtitle"] - msg["markup_subjective"] = msg["subjective"] + msg["markup"] = _make_hub_links(msg, "subtitle") + msg["markup_subjective"] = _make_hub_links(msg, "subjective") return msg +_word_split_re = re.compile(r'(\s+)') +_punctuation_re = re.compile( + '^(?P(?:%s)*)(?P.*?)(?P(?:%s)*)$' % ( + '|'.join(map(re.escape, ('(', '<', '<'))), + '|'.join(map(re.escape, ('.', ',', ')', '>', '\n', '>', "'s"))) + ) +) + + +def _make_hub_links(msg, attr): + if not msg.get(attr): + return "" + existing_usernames = [r[0] for r in User.query.values(User.username)] + usernames = [u for u in msg["usernames"] if u in existing_usernames] + words = _word_split_re.split(msg[attr]) + for i, word in enumerate(words): + match = _punctuation_re.match(word) + if match: + lead, middle, trail = match.groups() + if middle in usernames: + middle = '{}'.format( + flask.url_for("hub", name=middle), middle) + if lead + middle + trail != word: + words[i] = lead + middle + trail + return ''.join(words) + + class Feed(object): max_items = 100 diff --git a/hubs/tests/test_feed.py b/hubs/tests/test_feed.py index 29b21ba..042ff55 100644 --- a/hubs/tests/test_feed.py +++ b/hubs/tests/test_feed.py @@ -117,3 +117,29 @@ class FeedTest(APPTest): sorted(get_hubs_for_msg({"msg_id": "testmsg"})), ["infra", "ralph", "testhub"] ) + + def test_format_msg(self): + msg = { + "msg_ids": { + "testid1": {"msg_id": "testid1"}, + "testid2": {"msg_id": "testid2"}, + "testid3": {"msg_id": "testid3"}, + }, + "usernames": ["ralph", "decause"], + "subtitle": "ralph's ticket was commented by decause", + "subjective": "your ticket was commented by decause", + } + with app.test_request_context(): + result = format_msg(msg) + self.assertEqual( + result["dom_id"], + "067306d0b091ec48d9e2dded1ca9b1f8b1b2ac93") + self.assertEqual( + result["markup"], + """ralph's ticket was commented by """ + """decause""" + ) + self.assertEqual( + result["markup_subjective"], + """your ticket was commented by decause""" + ) From 5a753eca4a7c951e379441f3b4b42658f2f3edcc Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 20/35] Don't show the SSE error message when leaving the page --- diff --git a/hubs/static/client/app/components/feed/ItemsGetter.jsx b/hubs/static/client/app/components/feed/ItemsGetter.jsx index 75c50f9..dcfe7ca 100644 --- a/hubs/static/client/app/components/feed/ItemsGetter.jsx +++ b/hubs/static/client/app/components/feed/ItemsGetter.jsx @@ -11,6 +11,7 @@ export default class ItemsGetter extends React.Component { }; this.sseSource = null; this.setupSSESource = this.setupSSESource.bind(this); + this.tearDownSSESource = this.tearDownSSESource.bind(this); this.handleSSEEvent = this.handleSSEEvent.bind(this); this.handleSSEEventError = this.handleSSEEventError.bind(this); this.loadFromServer = this.loadFromServer.bind(this); @@ -23,17 +24,14 @@ export default class ItemsGetter extends React.Component { componentWillUnmount() { this.serverRequest.abort(); - if (this.sseSource) { - this.sseSource.removeEventListener( - this.props.sseEventName, this.this.props.handleSSEEvent); - this.sseSource.removeEventListener('error', this.handleSSEEventError); - } + this.tearDownSSESource(); + window.removeEventListener("beforeunload", this.tearDownSSESource); } setupSSESource() { - //if (!this.props.sseEventName || !this.props.handleSSEEvent) { - if (!this.props.sse.eventName || !this.props.sse.shouldReload) { - console.log("SSE auto-update is disabled"); + if (!this.props.sse || + !this.props.sse.eventName || + !this.props.sse.shouldReload) { // Auto-update is disabled. return; } @@ -47,6 +45,18 @@ export default class ItemsGetter extends React.Component { this.sseSource.addEventListener('error', this.handleSSEEventError); this.sseSource.addEventListener( this.props.sse.eventName, this.handleSSEEvent); + // This is necessary to avoid the error message being displayed right + // before the user changes page. + // Pretty much like: https://bugzilla.mozilla.org/show_bug.cgi?id=833462 + window.addEventListener("beforeunload", this.tearDownSSESource); + } + + tearDownSSESource() { + if (!this.sseSource) { return; } + this.sseSource.removeEventListener( + this.props.sse.eventName, this.handleSSEEvent); + this.sseSource.removeEventListener('error', this.handleSSEEventError); + this.sseSource = null; } handleSSEEvent(e) { @@ -55,7 +65,8 @@ export default class ItemsGetter extends React.Component { } } - handleSSEEventError() { + handleSSEEventError(e) { + console.log(e); this.setState({ sseError: ("Cannot auto-update the feed, you will have to refresh " +"the page manually to see new elements.") From 7d9541562bd5a9a18d52908c083047ab04a16155 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 21/35] Display a message when there is no feed item --- diff --git a/hubs/static/client/app/components/feed/Feed.jsx b/hubs/static/client/app/components/feed/Feed.jsx index 9292add..ec6b037 100644 --- a/hubs/static/client/app/components/feed/Feed.jsx +++ b/hubs/static/client/app/components/feed/Feed.jsx @@ -1,7 +1,21 @@ import React from 'react'; +import PropTypes from 'prop-types'; +import { + IntlProvider, + defineMessages, + FormattedMessage, + } from 'react-intl'; import Panel from './Panel.jsx'; +const messages = defineMessages({ + no_items: { + id: "hubs.components.feed.no_items", + defaultMessage: "No items yet.", + }, +}); + + export default class Feed extends React.Component { render() { @@ -12,13 +26,27 @@ export default class Feed extends React.Component { ); }); return ( -
    - {items} -
    + +
    + { (items.length == 0 && this.props.loaded) ? + + : + items + } +
    +     ); } } +Feed.propTypes = { + loaded: PropTypes.bool, +}; +Feed.defaultProps = { + loaded: true, +}; + + // vim: set ts=2 sw=2 et: diff --git a/hubs/static/client/app/widgets/feed/Widget.jsx b/hubs/static/client/app/widgets/feed/Widget.jsx index 636daa2..b266609 100644 --- a/hubs/static/client/app/widgets/feed/Widget.jsx +++ b/hubs/static/client/app/widgets/feed/Widget.jsx @@ -9,12 +9,16 @@ export default class Widget extends React.Component { super(props); this.state = { items: [], + loaded: false, }; this.handleServerData = this.handleServerData.bind(this); } handleServerData(data) { - this.setState({items: data}); + this.setState({ + items: data, + loaded: true, + }); } render() { @@ -29,6 +33,7 @@ export default class Widget extends React.Component { > ); From 6212bfe0ddad80609021593ed11ccfbcbf0c872a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 22/35] Handle error cases better in the SSE server --- diff --git a/hubs/backend/sse_server.py b/hubs/backend/sse_server.py index 781ddb2..7bcd7c7 100644 --- a/hubs/backend/sse_server.py +++ b/hubs/backend/sse_server.py @@ -9,7 +9,7 @@ import txredisapi as redis from twisted.application import service from twisted.web import server, resource -from twisted.internet import reactor, defer +from twisted.internet import reactor, defer, task from twisted.logger import ( Logger, globalLogBeginner, ILogObserver, formatEvent) from zope.interface import provider @@ -102,6 +102,7 @@ class Subscribe(resource.Resource): self.subscribers[target] = [] # first request to this target. log.debug("New subscriber: {req}", req=request) self.subscribers[target].append(request) + self._send_event(request, "connected", "null") log.debug("There is now {count} subscribers to {target}.", target=target, count=len(self.subscribers[target])) @@ -116,11 +117,25 @@ class Subscribe(resource.Resource): log.debug("Broadcasting to {num} requests.", num=len(target_subscribers)) for request in target_subscribers: - if event: - request.write("event: {}\r\n".format(event).encode("utf-8")) - request.write("data: {}\r\n".format(data).encode("utf-8")) - # The last CRLF is required to dispatch the event to the client. - request.write(b"\r\n") + self._send_event(request, event, data) + + def keepalive(self): + all_subscribers = [] + for subs in self.subscribers.values(): + all_subscribers.extend(subs) + if not all_subscribers: + return + log.debug("Broadcasting a ping to {num} requests.", + num=len(all_subscribers)) + for request in all_subscribers: + self._send_event(request, "ping", "null") + + def _send_event(self, request, event, data): + if event: + request.write("event: {}\r\n".format(event).encode("utf-8")) + request.write("data: {}\r\n".format(data).encode("utf-8")) + # The last CRLF is required to dispatch the event to the client. + request.write(b"\r\n") def request_closed(self, err, request, target): log.debug("Removing subscriber {subscriber} from {target}.", @@ -181,12 +196,18 @@ def main(): print(formatEvent(event)) globalLogBeginner.beginLoggingTo( [printingObserver], redirectStandardIO=False) + # Web service sub = Subscribe() site = server.Site(sub) port = int(hubs.app.app.config.get("SSE_URL", {}).get("port", 8080)) reactor.listenTCP(port, site) + # Redis client redisclient = RedisClientService(sub) redisclient.startService() + # Keepalive ping + keepalive = task.LoopingCall(sub.keepalive) + keepalive.start(30) + # Start the reactor reactor.run() diff --git a/hubs/backend/sse_server.tac b/hubs/backend/sse_server.tac index 57dc41f..1ea980f 100644 --- a/hubs/backend/sse_server.tac +++ b/hubs/backend/sse_server.tac @@ -13,7 +13,7 @@ which twistd will look for from __future__ import unicode_literals from twisted.application import service, internet -from twisted.internet import reactor +from twisted.internet import reactor, task from twisted.web import server import hubs.app @@ -37,3 +37,7 @@ web_service.setServiceParent(application) # Redis client redis_service = RedisClientService(broadcaster) redis_service.setServiceParent(application) + +# Keepalive ping +keepalive = task.LoopingCall(broadcaster.keepalive) +keepalive.start(30) diff --git a/hubs/static/client/app/components/feed/ItemsGetter.jsx b/hubs/static/client/app/components/feed/ItemsGetter.jsx index dcfe7ca..ca6397d 100644 --- a/hubs/static/client/app/components/feed/ItemsGetter.jsx +++ b/hubs/static/client/app/components/feed/ItemsGetter.jsx @@ -14,6 +14,7 @@ export default class ItemsGetter extends React.Component { this.tearDownSSESource = this.tearDownSSESource.bind(this); this.handleSSEEvent = this.handleSSEEvent.bind(this); this.handleSSEEventError = this.handleSSEEventError.bind(this); + this.handleSSEEventOpen = this.handleSSEEventOpen.bind(this); this.loadFromServer = this.loadFromServer.bind(this); } @@ -42,6 +43,7 @@ export default class ItemsGetter extends React.Component { return; } + this.sseSource.addEventListener('open', this.handleSSEEventOpen); this.sseSource.addEventListener('error', this.handleSSEEventError); this.sseSource.addEventListener( this.props.sse.eventName, this.handleSSEEvent); @@ -65,12 +67,24 @@ export default class ItemsGetter extends React.Component { } } + handleSSEEventOpen(e) { + this.setState({sseError: false}); + } + handleSSEEventError(e) { console.log(e); - this.setState({ - sseError: ("Cannot auto-update the feed, you will have to refresh " - +"the page manually to see new elements.") - }); + if (this.sseSource.readyState === 2) { + // Connection closed. + this.setState({ + sseError: ("Cannot auto-update the feed, you will have to refresh " + +"the page manually to see new elements.") + }); + } else if (this.sseSource.readyState === 0) { + // Reconnecting + this.setState({ + sseError: "Reconnecting to the auto-update source...", + }); + } } loadFromServer() { From c40253cb4e34b947994a4c766cf58df84552fdb2 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jun 22 2017 18:23:02 +0000 Subject: [PATCH 23/35] Fix usage of the session --- diff --git a/hubs/app.py b/hubs/app.py index 1cc6c68..c24a9e0 100644 --- a/hubs/app.py +++ b/hubs/app.py @@ -67,28 +67,34 @@ def check_auth(): flask.g.fedmsg_config = fedmsg_config if OIDC.user_loggedin: - if not hasattr(flask.session, 'auth') or not flask.session.auth: - flask.session.auth = munch.Munch( - fullname=OIDC.user_getfield('name'), - nickname=(OIDC.user_getfield('nickname') or - OIDC.user_getfield('sub')), - email=OIDC.user_getfield('email'), - timezone=OIDC.user_getfield('zoneinfo'), + if "auth" not in flask.session or not flask.session["auth"]: + user_info = OIDC.user_getinfo([ + "name", "nickname", "sub", "email", "zoneinfo", "cla", + "groups", + ]) + flask.session["auth"] = dict( + fullname=user_info['name'], + nickname=(user_info['nickname'] or user_info['sub']), + email=user_info['email'], + timezone=user_info['zoneinfo'], cla_done=('http://admin.fedoraproject.org/accounts/cla/done' - in OIDC.user_getfield('cla')), - groups=OIDC.user_getfield('groups'), + in user_info['cla']), + groups=user_info['groups'], logged_in=True, ) - flask.session.auth.avatar = username2avatar( - flask.session.auth.nickname) - - user = hubs.models.User.get_or_create( - flask.g.db, username=flask.session.auth.nickname, - fullname=flask.session.auth.fullname) - flask.session.auth.user = user - flask.g.auth = flask.session.auth + flask.session["auth"]["avatar"] = username2avatar( + flask.session["auth"]["nickname"]) + # Changes on mutable objects aren't picked up: + # http://flask.pocoo.org/docs/0.12/api/#flask.session.modified + flask.session.modified = True + flask.g.auth = munch.Munch(**flask.session["auth"]) + user = hubs.models.User.get_or_create( + flask.g.db, username=flask.session["auth"]["nickname"], + fullname=flask.session["auth"]["fullname"]) + flask.g.user = user else: - flask.g.auth = munch.Munch(logged_in=False, user=None) + flask.g.auth = munch.Munch(logged_in=False) + flask.g.user = None # Register widgets diff --git a/hubs/templates/hubs.html b/hubs/templates/hubs.html index 611fae9..63946a3 100644 --- a/hubs/templates/hubs.html +++ b/hubs/templates/hubs.html @@ -22,7 +22,7 @@
    - {% if hub.allows(g.auth.user, "config") %} + {% if hub.allows(g.user, "config") %}