PR#327: Let widgets declare their add and edit config urls - fedora-hubs

		`@@ -360,6 +360,10 @@`
		`def module(self):`
		`return hubs.widgets.registry[self.plugin]`

		`+ @property`
		`+ def edit_url(self):`
		`+ return self.module.get_edit_url(self.hub, self)`
		`+`

		`class User(BASE):`
		`__tablename__ = 'users'`

hubs/static/css/style.css

file modified

		`@@ -366,6 +366,7 @@`
		`.p-abs {`
		`position: absolute;`
		`bottom: 0;`
		`+ padding-left: 0;`
		`}`

		`#save_edits_btn {`

hubs/templates/add_widget.html

file modified

+3 -62

		`@@ -1,40 +1,21 @@`

		`<div class="modal-dialog" id="adding_form">`
		`<div class="modal-content">`
		`- {% if widget %}`
		`- <form method="post" action="{{ url_to }}">`
		`- {% endif %}`
		`+ <form method="post" action="{{ url_for('hub_add_widget', name=hub.name) }}">`
		`+ <input type="hidden" name="position" value="{{ position }}" />`
		`<!-- Modal content-->`
		`<div class="modal-header">`
		`<button type="button" class="close" data-dismiss="modal">×</button>`
		`<h4 class="modal-title">`
		`- Adding {% if widget %}widget "{{ widget.name }}"{% else %}a widget{% endif %} to hub: {{ hub.name }}`
		`+ Adding a widget to hub: {{ hub.name }}`
		`</h4>`
		`</div>`
		`<div class="modal-body">`
		`- {% if widgets %}`
		`<select id="widget" class="c-select" name="widget">`
		`{% for w in widgets %}`
		`<option value="{{ w }}">{{ w }}</option>`
		`{% endfor %}`
		`</select>`
		`- {% elif widget %}`
		`- <input name="widget_name" type="hidden" value="{{ widget.name }}">`
		`- <input name="position" type="hidden" value="{{ side }}">`
		`- {% for param in widget.get_parameters() %}`
		`- <fieldset class="form-group ">`
		`- <strong>{{ param.label \| capitalize }}</strong>`
		`- <input id="{{ param.name }}" class="form-control" type="text"`
		`- value="{{ param.validator.to_string(param.default) }}"`
		`- name="{{ param.name }}" />`
		`- {% if param.help %}`
		`- <small class="text-muted">{{ param.help }}</small>`
		`- {% endif %}`
		`- </fieldset>`
		`- {% else %}`
		`- <p>Nothing to configure</p>`
		`- {% endfor %}`
		`- {% endif %}`
		`</div>`
		`<div class="modal-footer">`
		`<button type="button" class="btn btn-default" data-dismiss="modal">`
		`@@ -44,46 +25,6 @@`
		`Add`
		`</button>`
		`</div>`
		`- {% if widget %}`
		`</form>`
		`- {% endif %}`
		`</div>`
		`</div>`
		`-`
		`- {% if widgets %}`
		`- <script type="text/javascript">`
		`- $("#submit_btn").unbind();`
		`- $("#submit_btn").click(function() {`
		`- console.log($('#widget').val());`
		`-`
		`- $.ajax({`
		`- url: 'add/' + $('#widget').val() + '?position={{ side }}',`
		`- dataType: 'html',`
		`- success: function(html) {`
		`- $('#adding_form').html(html);`
		`- },`
		`- error: function() {`
		`- $('#adding_form').html(`
		`- '<div class="modal-dialog"><div class="modal-content"> \`
		`- <div class="modal-body"> \`
		`- An error occured when trying to add a new widget\`
		`- </div></div></div>'`
		`- );`
		`- console.log('error');`
		`- console.trace();`
		`- },`
		`- });`
		`- return false;`
		`-`
		`- });`
		`- </script>`
		`- {% else %}`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/jquery-1.10.2.min.js') }}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static',filename='fedora-bootstrap/fedora-bootstrap.js')}}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/bootstrap3-typeahead.min.js') }}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/hubs_input.js') }}"></script>`
		`- {% endif %}`

hubs/templates/hubs.html

file modified

+50 -13

		`@@ -146,7 +146,7 @@`
		`<div class="card">`
		`<div class="card-block">`
		`<h4>`
		`- <a href="#" class="add_widget" data-position="left">`
		`+ <a href="{{ url_for("hub_add_widget", name=hub.name) }}?position=left" class="add_widget">`
		`<span><i class="fa fa-plus" aria-hidden="true"></i></span> Add a widget`
		`</a>`
		`</h4>`
		`@@ -164,7 +164,7 @@`
		`<div class="card">`
		`<div class="card-block">`
		`<h4>`
		`- <a href="#" class="add_widget" data-position="right">`
		`+ <a href="{{ url_for("hub_add_widget", name=hub.name) }}?position=right" class="add_widget">`
		`<span><i class="fa fa-plus" aria-hidden="true"></i></span> Add a widget`
		`</a>`
		`</h4>`
		`@@ -223,19 +223,16 @@`
		`};`

		`function setup_add_btns() {`
		`- $(".add_widget").unbind();`
		`- $(".add_widget").click(function() {`
		`- console.log($(this));`
		`- var _pos = $(this).attr('data-position');`
		`-`
		`+ // Setup events for the "add widget" button.`
		`+ $(".add_widget").click(function(e) {`
		`+ e.preventDefault();`
		`$.ajax({`
		`- url: 'add?position=' + _pos,`
		`+ url: $(this).attr('href'),`
		`dataType: 'html',`
		`success: function(html) {`
		`$('#edit_modal_content').html(html);`
		`- $('#edit_modal').modal();`
		`},`
		`- error: function() {`
		`+ error: function(html) {`
		`$('#edit_modal_content').html(`
		`'<div class="modal-dialog"><div class="modal-content"> \`
		`<div class="modal-body"> \`
		`@@ -244,11 +241,51 @@`
		`);`
		`console.log('error');`
		`console.trace();`
		`- $('#edit_modal').modal();`
		`+ console.log(html);`
		`},`
		`+ complete: function() {`
		`+ $('#edit_modal').modal();`
		`+ }`
		`});`
		`return false;`
		`});`
		`+ // Setup events for the "add widget" form using delegated events because it's`
		`+ // not in the DOM yet.`
		`+ $("body").on("submit", "#adding_form form", function(e) {`
		`+ var form = $(this);`
		`+ e.preventDefault();`
		`+ $.ajax({`
		`+ type: "POST",`
		`+ url: form.attr("action"),`
		`+ dataType: "json",`
		`+ data: form.serialize(),`
		`+ success: function(data) {`
		`+ if (data.status === "CONFIGURE") {`
		`+ $("#adding_form").load(data.url);`
		`+ } else if (data.status === "ADDED") {`
		`+ // no config necessary, reload the page`
		`+ window.location = window.location;`
		`+ } else {`
		`+ $('#adding_form .modal-body').html(`
		`+ "An error occured when trying to add a new widget."`
		`+ );`
		`+ $('#adding_form .modal-footer button[type="submit"]').remove();`
		`+ console.log('error');`
		`+ console.trace();`
		`+ console.log(data);`
		`+ }`
		`+ },`
		`+ error: function(html) {`
		`+ $('#adding_form .modal-body').html(`
		`+ "An error occured when trying to add a new widget."`
		`+ );`
		`+ $('#adding_form .modal-footer button[type="submit"]').remove();`
		`+ console.log('error');`
		`+ console.trace();`
		`+ console.log(html);`
		`+ }`
		`+ });`
		`+ });`
		`}`

		`$('#save_edits_btn').click(function() {`
		`@@ -281,10 +318,10 @@`
		`function setup_edit_btns() {`
		`$("body").on("click", ".edit_widget", function(e) {`
		`e.preventDefault();`
		`- var _idx = $(this).attr('data-idx');`
		`+ var url = $(this).attr('data-url');`

		`$.ajax({`
		`- url: _idx + '/edit',`
		`+ url: url,`
		`dataType: 'html',`
		`success: function(html) {`
		`$('#edit_modal_content').html(html);`

hubs/templates/widget_add.html

file added

+37

		`@@ -0,0 +1,37 @@`
		`+ <div class="modal-dialog widget-add">`
		`+ <div class="modal-content">`
		`+ <form method="post" action="{{ url_for('widget_add', hub=hub.name, widget=widget.name) }}">`
		`+ <input type="hidden" name="position" value="{{ position }}" />`
		`+ <!-- Modal content-->`
		`+ <div class="modal-header">`
		`+ <button type="button" class="close" data-dismiss="modal">×</button>`
		`+ <h4 class="modal-title">`
		`+ Adding widget "{{ widget.name }}" to hub {{ hub.name }}`
		`+ </h4>`
		`+ </div> <!-- /modal-header -->`
		`+ <div class="modal-body">`
		`+ {% for param in widget.get_parameters() %}`
		`+ <fieldset class="form-group ">`
		`+ <strong>{{ param.label \| capitalize }}</strong>`
		`+ <input id="{{ param.name }}" class="form-control" type="text"`
		`+ value="{{ param.validator.to_string(param.default) }}"`
		`+ name="{{ param.name }}" />`
		`+ {% if param.help %}`
		`+ <small class="text-muted">{{ param.help }}</small>`
		`+ {% endif %}`
		`+ </fieldset>`
		`+ {% else %}`
		`+ <p>Nothing to configure</p>`
		`+ {% endfor %}`
		`+ </div> <!-- /modal-body -->`
		`+ <div class="modal-footer">`
		`+ <button type="button" class="btn btn-default" data-dismiss="modal">`
		`+ Cancel`
		`+ </button>`
		`+ <button type="submit" class="btn btn-primary">`
		`+ Add`
		`+ </button>`
		`+ </div> <!-- /modal-footer -->`
		`+ </form>`
		`+ </div>`
		`+ </div>`

hubs/templates/widget_edit.html ~~hubs/templates/edit.html~~

file renamed

+11 -21

		`@@ -1,18 +1,19 @@`
		`-`
		`- <div class="modal-dialog" id="edit_form">`
		`+ <div class="modal-dialog widget-edit">`
		`<div class="modal-content">`
		`- <form method="post" action="{{ url_to }}">`
		`+ <form method="post" action="{{ url_for('widget_edit', hub=hub.name, idx=widget_instance.idx) }}">`
		`<!-- Modal content-->`
		`<div class="modal-header">`
		`<button type="button" class="close" data-dismiss="modal">×</button>`
		`- <h4 class="modal-title">Configure {{ name }}</h4>`
		`- </div>`
		`+ <h4 class="modal-title">`
		`+ Configure {{ widget_instance.plugin }}`
		`+ </h4>`
		`+ </div> <!-- /modal-header -->`
		`<div class="modal-body">`
		`- {% for param in widget.module.get_parameters() %}`
		`+ {% for param in widget_instance.module.get_parameters() %}`
		`<fieldset class="form-group ">`
		`<strong>{{ param.label \| capitalize }}</strong>`
		`<input id="{{ param.name }}" class="form-control" type="text"`
		`- value="{{ param.validator.to_string(widget.config.get(param.name, param.default)) }}"`
		`+ value="{{ param.validator.to_string(widget_instance.config.get(param.name, param.default)) }}"`
		`name="{{ param.name }}" />`
		`{% if param.help %}`
		`<small class="text-muted">{{ param.help }}</small>`
		`@@ -21,20 +22,18 @@`
		`{% else %}`
		`<p>Nothing to configure</p>`
		`{% endfor %}`
		`- </div>`
		`+ </div> <!-- /modal-body -->`
		`<div class="modal-footer">`
		`<button type="button" class="btn btn-default" data-dismiss="modal">`
		`Close`
		`</button>`
		`- {% if widget.module.get_parameters() %}`
		`<button type="submit" class="btn btn-primary">`
		`Save`
		`</button>`
		`- {% endif %}`
		`- </div>`
		`+ </div> <!-- /modal-footer -->`
		`</form>`
		`<form method="POST" action="{{ url_for(`
		`- 'widget_edit_delete', hub=widget.hub.name, idx=widget.idx) }}"`
		`+ 'widget_edit_delete', hub=hub.name, idx=widget_instance.idx) }}"`
		`class="modal-footer p-abs" >`
		`<button type="submit" class="btn btn-danger" id="delete_widget">`
		`Remove this widget`
		`@@ -42,12 +41,3 @@`
		`</form>`
		`</div>`
		`</div>`
		`-`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/jquery-1.10.2.min.js') }}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static',filename='fedora-bootstrap/fedora-bootstrap.js')}}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/bootstrap3-typeahead.min.js') }}"></script>`
		`- <script type="text/javascript" src="{{`
		`- url_for('static', filename='js/hubs_input.js') }}"></script>`

hubs/tests/test_fedora_hubs_flask_api.py

file modified

+54 -41

		`@@ -2,11 +2,8 @@`

		`import unittest`
		`from six.moves.urllib.parse import urlparse`
		`- import ast # import for string to dict conversion without unicode`

		`from flask import json`
		`- from os.path import dirname`
		`- import vcr`
		`from mock import mock`
		`from werkzeug.datastructures import ImmutableMultiDict`

		`@@ -16,8 +13,6 @@`

		`import hubs.models`

		`- import fedmsg.config`
		`-`

		`class HubsAPITest(hubs.tests.APPTest):`
		`def test_index_logged_out(self):`
		`@@ -214,30 +209,42 @@`
		`"/openidc/Authorization")`

		`def test_hub_add_widget_get_no_args(self):`
		`- result = self.app.get('/ralph/add', follow_redirects=False)`
		`+ user = tests.FakeAuthorization('ralph')`
		`+ with tests.auth_set(app, user):`
		`+ result = self.app.get('/ralph/add', follow_redirects=False)`
		`self.assertEqual(result.status_code, 400)`
		`expected_str = 'Invalid position provided'`
		`self.assertIn(expected_str, result.get_data(as_text=True))`

		`def test_hub_add_widget_get_with_args(self):`
		`- result = self.app.get('/ralph/add?position=right',`
		`- follow_redirects=True)`
		`+ user = tests.FakeAuthorization('ralph')`
		`+ with tests.auth_set(app, user):`
		`+ result = self.app.get('/ralph/add?position=right',`
		`+ follow_redirects=True)`
		`self.assertEqual(result.status_code, 200)`
		`- expected_str = 'Adding a widget to hub: ralph'`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`- expected_str = "url: 'add/' + $('#widget').val() + '?position=right',"`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`+ page_html = result.get_data(as_text=True)`
		`+ self.assertIn('Adding a widget to hub: ralph', page_html)`
		`+ self.assertIn('<form method="post" action="/ralph/add">', page_html)`
		`+ self.assertIn('<input type="hidden" name="position" value="right" />',`
		`+ page_html)`

		`def test_hub_add_widget_post_no_widget_name(self):`
		`- data = {}`
		`- result = self.app.post('/ralph/add', data=data, follow_redirects=False)`
		`+ data = {"position": "left"}`
		`+ user = tests.FakeAuthorization('ralph')`
		`+ with tests.auth_set(app, user):`
		`+ result = self.app.post(`
		`+ '/ralph/add', data=data, follow_redirects=False)`
		`self.assertEqual(result.status_code, 400)`
		`expected_str = 'Invalid request sent'`
		`self.assertIn(expected_str, result.get_data(as_text=True))`

		`def test_hub_add_widget_post_invalid_widget_name(self):`
		`- data = {'widget_name': 'invalid_widget_name'}`
		`- result = self.app.post('/ralph/add', data=data, follow_redirects=False)`
		`+ data = {'widget': 'invalid_widget_name',`
		`+ 'position': 'right'}`
		`+ user = tests.FakeAuthorization('ralph')`
		`+ with tests.auth_set(app, user):`
		`+ result = self.app.post(`
		`+ '/ralph/add', data=data, follow_redirects=False)`
		`self.assertEqual(result.status_code, 404)`
		`expected_str = 'Unknown widget called'`
		`self.assertIn(expected_str, result.get_data(as_text=True))`
		`@@ -245,31 +252,43 @@`
		`def test_hub_add_widget_post_valid_widget_name_no_args(self):`
		`user = tests.FakeAuthorization('ralph')`
		`with tests.auth_set(app, user):`
		`- data = {'widget_name': 'about'}`
		`+ data = {`
		`+ 'widget': 'memberships',`
		`+ 'position': 'right',`
		`+ }`
		`result = self.app.post('/ralph/add', data=data)`
		`- self.assertEqual(result.status_code, 302)`
		`- self.assertEqual(urlparse(result.location).path, "/ralph/edit")`
		`+ self.assertEqual(result.status_code, 200)`
		`+ self.assertEqual(`
		`+ json.loads(result.get_data(as_text=True)),`
		`+ {"status": "ADDED"})`
		`result = self.app.get('/ralph/edit')`
		`self.assertEqual(result.status_code, 200)`
		`- expected_str = '<h6 class="dropdown-header">Account Information</h6>'`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`- expected_str = '<a href="#" class="dropdown-item">Full Name: fullname: ralph</a>'`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`+ page_html = result.get_data(as_text=True)`
		`+ self.assertIn('data-url="/ralph/w/memberships/', page_html)`

		`def test_hub_add_widget_post_valid_widget_name_with_args(self):`
		`+ self.assertEqual(`
		`+ hubs.models.Widget.query.filter(`
		`+ hubs.models.Hub.name == "ralph",`
		`+ hubs.models.Widget.plugin == "about",`
		`+ ).count(), 1)`
		`user = tests.FakeAuthorization('ralph')`
		`with tests.auth_set(app, user):`
		`- data = {'widget_name': 'about', 'text': 'text of widget'}`
		`- result = self.app.post('/ralph/add', data=data,`
		`+ data = {`
		`+ 'text': 'text of widget',`
		`+ 'position': 'right',`
		`+ }`
		`+ result = self.app.post('/ralph/add/about', data=data,`
		`follow_redirects=False)`
		`- self.assertEqual(result.status_code, 302)`
		`- self.assertEqual(urlparse(result.location).path, "/ralph/edit")`
		`- result = self.app.get('/ralph/edit')`
		`self.assertEqual(result.status_code, 200)`
		`- expected_str = '<h6 class="dropdown-header">Account Information</h6>'`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`- expected_str = '<a href="#" class="dropdown-item">Full Name: fullname: ralph</a>'`
		`- self.assertIn(expected_str, result.get_data(as_text=True))`
		`+ self.assertEqual(`
		`+ json.loads(result.get_data(as_text=True)),`
		`+ {"status": "ADDED"})`
		`+ self.assertEqual(`
		`+ hubs.models.Widget.query.filter(`
		`+ hubs.models.Hub.name == "ralph",`
		`+ hubs.models.Widget.plugin == "about",`
		`+ ).count(), 2)`

		`def test_hub_edit_widget_get_logged_in(self):`
		`user = tests.FakeAuthorization('ralph')`
		`@@ -291,7 +310,7 @@`
		`url = '/ralph/37/edit'`
		`result = self.app.post(url, data=data, follow_redirects=False)`
		`self.assertEqual(result.status_code, 302)`
		`- self.assertEqual(urlparse(result.location).path, '/ralph/')`
		`+ self.assertEqual(urlparse(result.location).path, '/ralph/edit')`

		`def test_hub_visit_counter_logged_in(self):`
		`user = tests.FakeAuthorization('ralph')`
		`@@ -342,7 +361,7 @@`
		`with tests.auth_set(app, user):`
		`url = '/ralph/add/about?position=right'`
		`result = self.app.get(url)`
		`- self.assertIn('Adding widget "about" to hub: ralph',`
		`+ self.assertIn('Adding widget "about" to hub ralph',`
		`result.get_data(as_text=True))`

		`def test_hub_add_widget_invalid_side(self):`
		`@@ -376,7 +395,7 @@`
		`self.assertEqual(result.status_code, 404)`

		`def mocked_requests_get(args, *kwargs):`
		`- class MockResponse:`
		`+ class MockResponse: # flake8: noqa`
		`def __init__(self, json_data, status_code):`
		`self.json_data = json_data`
		`self.status_code = status_code`
		`@@ -491,9 +510,3 @@`
		`result = self.app.get(url)`
		`self.assertEqual(result.status_code, 403)`
		`self.assertEqual(result.get_data(as_text=True), 'User does not exist')`
		`-`
		`-`
		`-`
		`-`
		`- if __name__ == '__main__':`
		`- unittest.main()`

hubs/tests/test_widgets/test_about.py

file modified

+1 -1

		`@@ -12,7 +12,7 @@`

		`def test_data_simple(self):`
		`widget = self.widget_instance('ralph', self.plugin)`
		`- response = self.app.get('/ralph/%i/json/' % widget.idx)`
		`+ response = self.app.get('/ralph/%i/json' % widget.idx)`
		`assert response.status_code == 200, response.status_code`
		`data = json.loads(response.get_data(as_text=True))`
		`self.assertDictEqual(data['data'], {`

hubs/tests/test_widgets/test_badges.py

file modified

+1 -1

		`@@ -11,7 +11,7 @@`

		`def test_data_simple(self):`
		`widget = self.widget_instance('ralph', self.plugin)`
		`- response = self.app.get('/ralph/%i/json/' % widget.idx)`
		`+ response = self.app.get('/ralph/%i/json' % widget.idx)`
		`assert response.status_code == 200, response.status_code`
		`data = json.loads(response.get_data(as_text=True))`
		`self.assertEquals(data['plugin'], 'badges')`

hubs/tests/test_widgets/test_fedmsgstats.py

file modified

+1 -1

		`@@ -10,7 +10,7 @@`

		`def test_data_simple(self):`
		`widget = self.widget_instance('ralph', self.plugin)`
		`- response = self.app.get('/ralph/%i/json/' % widget.idx)`
		`+ response = self.app.get('/ralph/%i/json' % widget.idx)`
		`assert response.status_code == 200, response.status_code`
		`data = json.loads(response.get_data(as_text=True))`
		`self.assertDictEqual(data['data'], {`

hubs/tests/test_widgets/test_library.py

file modified

+1 -1

		`@@ -10,7 +10,7 @@`

		`def test_data_simple(self):`
		`widget = self.widget_instance('ralph', self.plugin)`
		`- response = self.app.get('/ralph/%i/json/' % widget.idx)`
		`+ response = self.app.get('/ralph/%i/json' % widget.idx)`
		`self.assertEqual(response.status_code, 200)`
		`data = json.loads(response.get_data(as_text=True))`
		`expected_dict = {`

hubs/tests/test_widgets/test_meetings.py

file modified

+1 -1

		`@@ -11,7 +11,7 @@`
		`def test_data_simple(self):`
		`team = 'i18n'`
		`widget = self.widget_instance(team, self.plugin)`
		`- response = self.app.get('/%s/%i/json/' % (team, widget.idx))`
		`+ response = self.app.get('/%s/%i/json' % (team, widget.idx))`
		`self.assertEqual(200, response.status_code)`
		`data = json.loads(response.get_data(as_text=True))`
		`calendar_name = data['data']['calendar']`

hubs/views/hub.py

file modified

+2 -95

		`@@ -5,7 +5,7 @@`
		`import hubs.models`

		`from hubs.app import app, session`
		`- from .utils import get_hub`
		`+ from .utils import get_hub, login_required`


		`@app.route('/<name>')`
		`@@ -25,8 +25,8 @@`
		`return response`


		`- @app.route('/<name>/edit/', methods=['GET', 'POST'])`
		`@app.route('/<name>/edit', methods=['GET', 'POST'])`
		`+ @login_required`
		`def hub_edit(name):`
		`if flask.request.method == 'POST':`
		`return hub_edit_post(name)`
		`@@ -139,96 +139,3 @@`
		`return ('ok', 200)`
		`else:`
		`return flask.redirect(flask.url_for('hub', name=name))`
		`-`
		`-`
		`- @app.route('/<name>/add/', methods=['GET', 'POST'])`
		`- @app.route('/<name>/add', methods=['GET', 'POST'])`
		`- def hub_add_widgets(name):`
		`- if flask.request.method == 'POST':`
		`- return hub_add_widget_post(name)`
		`- else:`
		`- return hub_add_widget_get(name)`
		`-`
		`-`
		`- @app.route('/<name>/add/<widget_name>/', methods=['GET'])`
		`- @app.route('/<name>/add/<widget_name>', methods=['GET'])`
		`- def hub_add_widget(name, widget_name):`
		`- hub = get_hub(name)`
		`- side = str(flask.request.args.get('position')).lower()`
		`- if side not in ['right', 'left']:`
		`- flask.abort(400, 'Invalid position provided')`
		`-`
		`- widget = hubs.widgets.registry[widget_name]`
		`- if widget.position not in ['both', side]:`
		`- flask.abort(400, "This widget can't be placed here")`
		`-`
		`- return flask.render_template(`
		`- 'add_widget.html',`
		`- hub=hub,`
		`- widget=widget,`
		`- side=side,`
		`- url_to=flask.url_for('hub_add_widgets', name=name),`
		`- )`
		`-`
		`-`
		`- def hub_add_widget_get(name):`
		`- hub = get_hub(name)`
		`- side = str(flask.request.args.get('position')).lower()`
		`- if side not in ['right', 'left']:`
		`- flask.abort(400, 'Invalid position provided')`
		`-`
		`- widgets = [`
		`- widget`
		`- for widget in hubs.widgets.registry`
		`- if hubs.widgets.registry[widget].position in ['both', side]`
		`- ]`
		`- return flask.render_template(`
		`- 'add_widget.html',`
		`- hub=hub,`
		`- widgets=widgets,`
		`- side=side,`
		`- )`
		`-`
		`-`
		`- def hub_add_widget_post(name):`
		`- widget_name = flask.request.form.get('widget_name')`
		`- position = flask.request.form.get('position', '').lower()`
		`- if not widget_name:`
		`- flask.abort(400, 'Invalid request sent')`
		`- if widget_name not in hubs.widgets.registry:`
		`- flask.abort(404, 'Unknown widget called')`
		`-`
		`- hub = get_hub(name)`
		`-`
		`- widget = widget = hubs.models.Widget(`
		`- plugin=widget_name, index=-1, left=position == 'left')`
		`- error = False`
		`- config = {}`
		`- for param in widget.module.get_parameters():`
		`- val = flask.request.form.get(param.name)`
		`- if not val:`
		`- flask.flash(`
		`- 'You must provide a value for: %s' % param.name, 'error')`
		`- error = True`
		`- break`
		`- try:`
		`- val = param.validator.from_string(val)`
		`- config[param.name] = val`
		`- except Exception as err:`
		`- flask.flash('Invalid data provided, error: %s' % err, 'error')`
		`- error = True`
		`- if not error:`
		`- widget.hub = hub`
		`- widget.config = config`
		`- try:`
		`- flask.g.db.add(widget)`
		`- flask.g.db.flush()`
		`- widget.hub.last_edited = datetime.datetime.utcnow()`
		`- flask.g.db.commit()`
		`- except Exception as err:`
		`- print(err)`
		`- flask.flash(`
		`- 'Could not save the configuration to the database '`
		`- 'if the error persists, please warn an admin',`
		`- 'error')`
		`- return flask.redirect(flask.url_for('hub_edit', name=hub.name))`

hubs/views/root.py

file modified

		`@@ -50,6 +50,7 @@`

		`hubs.models.User.get_or_create(`
		`flask.g.db, username=flask.g.auth.nickname, fullname=flask.g.auth.fullname)`
		`+ flask.flash('Login successful', 'success')`

		`return flask.redirect(return_point)`

hubs/views/utils.py

file modified

+74 -1

		`@@ -1,13 +1,18 @@`
		`from __future__ import unicode_literals`

		`+ import datetime`
		`import flask`
		`import functools`
		`import hubs.models`
		`+ import logging`

		`from six.moves.urllib import parse as urlparse`
		`from sqlalchemy.orm.exc import NoResultFound`


		`+ log = logging.getLogger(__name__)`
		`+`
		`+`
		`def get_hub(name, session=None):`
		`""" Utility shorthand to get a hub and 404 if not found. """`
		`if session is None:`
		`@@ -38,6 +43,67 @@`
		`flask.abort(404)`


		`+ class WidgetConfigError(Exception):`
		`+ pass`
		`+`
		`+`
		`+ def create_widget_instance(hub, widget, position):`
		`+ """View helper to instanciate a widget on a hub.`
		`+`
		`+ Arguments:`
		`+ hub (hubs.models.Hub): Instance of the hub to create the widget in.`
		`+ widget (hubs.widgets.base.Widget): Widget to instanciate.`
		+ position (str): either ``left`` or ``right``.
		`+ """`
		`+ widget_instance = hubs.models.Widget(`
		`+ hub=hub, plugin=widget.name, index=-1,`
		`+ left=(position == 'left'))`
		`+ flask.g.db.add(widget_instance)`
		`+ flask.g.db.flush() # will populate Widget.config`
		`+ try:`
		`+ configure_widget_instance(widget_instance)`
		`+ except WidgetConfigError as e:`
		`+ flask.flash(e.args[0], "error")`
		`+ else:`
		`+ flask.flash("The widget has been added.")`
		`+`
		`+`
		`+ def configure_widget_instance(widget_instance):`
		`+ """Configure a widget instance using the request form data.`
		`+`
		`+ Arguments:`
		`+ widget_instance (hubs.models.Widget): Instance of the widget that is to`
		`+ be configured.`
		`+ """`
		`+ config = {}`
		`+ for param in widget_instance.module.get_parameters():`
		`+ val = flask.request.form.get(param.name)`
		`+ if not val:`
		`+ raise WidgetConfigError(`
		`+ 'You must provide a value for: %s' % param.name)`
		`+ try:`
		`+ config[param.name] = param.validator.from_string(val)`
		`+ except ValueError as err:`
		`+ raise WidgetConfigError('Invalid data provided, error: %s' % err)`
		`+ # Updating in-place is not supported, it's a class property.`
		`+ cur_config = widget_instance.config or {}`
		`+ cur_config = cur_config.copy()`
		`+ cur_config.update(config)`
		`+ widget_instance.config = cur_config`
		`+ # TODO: use a SQLAlchemy signal here`
		`+ widget_instance.hub.last_edited = datetime.datetime.utcnow()`
		`+ try:`
		`+ flask.g.db.commit()`
		`+ except Exception as err:`
		`+ log.warn(`
		`+ "Could not save the configuration of widget "`
		`+ "%s to the database: %s.",`
		`+ widget_instance.idx, err)`
		`+ raise WidgetConfigError(`
		`+ 'Could not save the configuration to the database '`
		`+ 'if the error persists, please warn an admin')`
		`+`
		`+`
		`def authenticated():`
		`""" Utility function checking if the current auth is set or not."""`
		`return hasattr(flask.g, 'auth') \`
		`@@ -52,7 +118,6 @@`
		`def decorated_function(args, *kwargs):`
		`""" Decorated function, actually does the work. """`
		`if not authenticated():`
		`- flask.flash('Login required', 'errors')`
		`return flask.redirect(flask.url_for(`
		`'login', next=flask.request.url))`

		`@@ -71,3 +136,11 @@`
		`return test_url.scheme in ('http', 'https') and \`
		`ref_url.netloc == test_url.netloc`

		`+`
		`+ def get_position():`
		`+ # Use request.values instead of request.form to allow the position to be`
		`+ # passed in the query string.`
		`+ position = flask.request.values.get('position', '')`
		`+ if position not in ['right', 'left']:`
		`+ flask.abort(400, 'Invalid position provided')`
		`+ return position`

hubs/views/widget.py

file modified

+76 -58

		`@@ -1,83 +1,97 @@`
		`from __future__ import unicode_literals, absolute_import`

		`- import datetime`
		`import flask`

		`from hubs.app import app`
		`+ from hubs.widgets import registry`
		`from pkg_resources import resource_isdir`
		`- from .utils import get_widget_instance`
		`+ from .utils import (`
		`+ create_widget_instance, configure_widget_instance, get_hub,`
		`+ get_widget_instance, login_required, WidgetConfigError,`
		`+ get_position,`
		`+ )`


		`- @app.route('/<hub>/<int:idx>/json')`
		`- @app.route('/<hub>/<int:idx>/json/')`
		`- def widget_json(hub, idx):`
		`- widget = get_widget_instance(hub, idx)`
		`- response = flask.jsonify(widget.__json__())`
		`- # TODO -- modify headers with response.headers['X-fedora-hubs-wat'] = 'foo'`
		`- return response`
		`+ @app.route('/<name>/add', methods=['GET', 'POST'])`
		`+ def hub_add_widget(name):`
		`+ hub = get_hub(name)`
		`+ position = get_position()`
		`+ widgets = [`
		`+ widget`
		`+ for widget in registry`
		`+ if registry[widget].position in ['both', position]`
		`+ ]`

		`-`
		`- @app.route('/<hub>/<int:idx>/edit/', methods=['GET', 'POST'])`
		`- @app.route('/<hub>/<int:idx>/edit', methods=['GET', 'POST'])`
		`- def widget_edit(hub, idx):`
		`if flask.request.method == 'POST':`
		`- return widget_edit_post(hub, idx)`
		`- else:`
		`- return widget_edit_get(hub, idx)`
		`+ widget_name = flask.request.form.get('widget')`
		`+ if not widget_name:`
		`+ flask.abort(400, 'Invalid request sent')`
		`+ if widget_name not in widgets:`
		`+ flask.abort(404, 'Unknown widget called')`
		`+ widget = registry[widget_name]`
		`+ if widget.get_parameters():`
		`+ return flask.jsonify({`
		`+ "status": "CONFIGURE",`
		`+ "url": widget.get_add_url(hub, position),`
		`+ })`
		`+ create_widget_instance(hub, widget, position)`
		`+ return flask.jsonify({"status": "ADDED"})`
		`+ return flask.render_template(`
		`+ 'add_widget.html',`
		`+ hub=hub,`
		`+ widgets=widgets,`
		`+ position=position,`
		`+ )`


		`- def widget_edit_get(hub, idx):`
		`- widget = get_widget_instance(hub, idx)`
		`+ @app.route('/<hub>/add/<widget>', methods=['GET', 'POST'])`
		`+ def widget_add(hub, widget):`
		`+ hub = get_hub(hub)`
		`+ position = get_position()`
		`+ try:`
		`+ widget = registry[widget]`
		`+ except KeyError:`
		`+ flask.abort(404, 'Unknown widget: %s' % widget)`
		`+ if flask.request.method == 'POST':`
		`+ create_widget_instance(hub, widget, position)`
		`+ return flask.jsonify({"status": "ADDED"})`
		`return flask.render_template(`
		`- 'edit.html',`
		`+ 'widget_add.html',`
		`hub=hub,`
		`widget=widget,`
		`- url_to=flask.url_for('widget_edit', hub=hub, idx=idx)`
		`+ position=position,`
		`)`


		`- def widget_edit_post(hub, idx):`
		`- widget = get_widget_instance(hub, idx)`
		`- error = False`
		`- config = {}`
		`- for param in widget.module.get_parameters():`
		`- val = flask.request.form.get(param.name)`
		`- if not val:`
		`- flask.flash(`
		`- 'You must provide a value for: %s' % param.name, 'error')`
		`- error = True`
		`- break`
		`- try:`
		`- val = param.validator.from_string(val)`
		`- config[param.name] = val`
		`- except Exception as err:`
		`- flask.flash('Invalid data provided, error: %s' % err, 'error')`
		`- error = True`
		`- if not error:`
		`- cur_config = widget.config`
		`- cur_config.update(config)`
		`- widget.config = cur_config`
		`- widget.hub.last_edited = datetime.datetime.utcnow()`
		`- flask.g.db.add(widget)`
		`+ @app.route('/<hub>/<int:idx>/edit', methods=['GET', 'POST'])`
		`+ @login_required`
		`+ def widget_edit(hub, idx):`
		`+ widget_instance = get_widget_instance(hub, idx)`
		`+ if flask.request.method == 'POST':`
		`try:`
		`- flask.g.db.commit()`
		`- except Exception as err:`
		`- flask.flash(`
		`- 'Could not save the configuration to the database '`
		`- 'if the error persists, please warn an admin',`
		`- 'error')`
		`- return flask.redirect(flask.url_for('hub', name=hub))`
		`+ configure_widget_instance(widget_instance)`
		`+ except WidgetConfigError as e:`
		`+ flask.flash(e.args[0], "error")`
		`+ else:`
		`+ flask.flash("The widget has been updated.")`
		`+ return flask.redirect(flask.url_for('hub_edit', name=hub))`
		`+ else:`
		`+ return flask.render_template(`
		`+ 'widget_edit.html',`
		`+ hub=widget_instance.hub,`
		`+ widget_instance=widget_instance,`
		`+ )`


		`- @app.route('/<hub>/<int:idx>/delete/', methods=['POST'])`
		`@app.route('/<hub>/<int:idx>/delete', methods=['POST'])`
		`+ @login_required`
		`def widget_edit_delete(hub, idx):`
		`''' Remove a widget from a hub. '''`
		`- widget = get_widget_instance(hub, idx)`
		`- flask.g.db.delete(widget)`
		`+ widget_instance = get_widget_instance(hub, idx)`
		`+ flask.g.db.delete(widget_instance)`
		`try:`
		`flask.g.db.commit()`
		`- except Exception as err:`
		`+ except Exception:`
		`flask.flash(`
		`'Could not delete this widget from this hub in the database '`
		`'if the error persists, please warn an admin',`
		`@@ -85,11 +99,16 @@`
		`return flask.redirect(flask.url_for('hub_edit', name=hub))`


		`- @app.route('/source/<name>/')`
		`+ @app.route('/<hub>/<int:idx>/json')`
		`+ def widget_json(hub, idx):`
		`+ widget = get_widget_instance(hub, idx)`
		`+ response = flask.jsonify(widget.__json__())`
		`+ # TODO -- modify headers with response.headers['X-fedora-hubs-wat'] = 'foo'`
		`+ return response`
		`+`
		`+`
		`@app.route('/source/<name>')`
		`def widget_source(name):`
		`- from hubs.widgets import registry`
		`-`
		`try:`
		`widget_path = registry[name].__module__`
		`except KeyError:`
		`@@ -102,4 +121,3 @@`
		`widget_url += ".py"`
		`url = "/".join([app.config.get('SOURCE_URL'), widget_url])`
		`return flask.redirect(url)`
		`-`

hubs/widgets/base.py

file modified

+40 -1

		`@@ -52,7 +52,7 @@`
		`"""`
		`The main widget class, you must subclass it to create a widget.`

		`- Arguments:`
		`+ Attributes:`
		`name (str): The widget name. It will not be displayed in the UI, but`
		`will appear in some URLs, so be careful to only use simple,`
		`URL-compatible characters.`
		`@@ -194,6 +194,45 @@`
		`self.name, url_rule.lstrip("/"))`
		`app.add_url_rule(rule, view_func=view_func)`

		`+ def get_add_url(self, hub, position):`
		`+ """Returns the URL to the configuration panel to add this widget.`
		`+`
		`+ By default, it returns the URL to the common widget configuration panel`
		`+ that will simply display the widget's parameters in a form. Widgets`
		`+ override this method to implement a more complex configuration view.`
		`+`
		`+ If a more complex configuration view is implemented, the resulting HTML`
		+ code will be wrapped in a ``<div class="modal-dialog">`` tag.
		`+`
		`+ Args:`
		`+ hub (hubs.models.Hub): The hub instance in the database.`
		`+ position (str): the position where the widget should be added. It`
		+ must be either ``left`` or ``right``, and must be in the
		+ supported positions for this widget (see :py:attr:`position`).
		`+`
		`+ Returns:`
		`+ The URL to add this widget, as a string.`
		`+ """`
		`+ return "%s?position=%s" % (`
		`+ flask.url_for('widget_add', hub=hub.name, widget=self.name),`
		`+ position)`
		`+`
		`+ def get_edit_url(self, hub, widget_instance):`
		`+ """Returns the URL to the configuration panel to edit this widget.`
		`+`
		+ This method works like :py:meth:`get_add_url`.
		`+`
		`+ Args:`
		`+ hub (hubs.models.Hub): The hub instance in the database.`
		`+ widget_instance (hubs.models.Widget): The widget instance in the`
		`+ database that must be configured.`
		`+`
		`+ Returns:`
		`+ The URL to edit this widget, as a string.`
		`+ """`
		`+ return flask.url_for(`
		`+ 'widget_edit', hub=hub.name, idx=widget_instance.idx)`
		`+`
		`def get_cached_functions(self):`
		`"""`
		`Returns:`

hubs/widgets/templates/panel.html

file modified

+1 -1

		`@@ -6,7 +6,7 @@`
		`<span><i class="fa fa-external-link" aria-hidden="true"></i></span>`
		`</a>`
		`<a data-target="#edit_modal" data-toggle="modal" type="button"`
		`- class="edit_widget" data-idx="{{ widget_instance.idx }}">`
		`+ class="edit_widget" data-url="{{ widget_instance.edit_url }}">`
		`<span><i class="fa fa-pencil" aria-hidden="true"></i></span>`
		`</a>`
		`</div>`

populate.py

file modified

+7 -7

		`@@ -15,13 +15,13 @@`

		`# Register widgets we will use`
		`hubs.widgets.registry.register_list([`
		`- "hubs.widgets.contact.Contact",`
		`- "hubs.widgets.stats.Stats",`
		`- "hubs.widgets.rules.Rules",`
		`- "hubs.widgets.meetings.Meetings",`
		`- "hubs.widgets.about.About",`
		`- "hubs.widgets.sticky.Sticky",`
		`- "hubs.widgets.dummy.Dummy",`
		`+ "hubs.widgets.contact:Contact",`
		`+ "hubs.widgets.stats:Stats",`
		`+ "hubs.widgets.rules:Rules",`
		`+ "hubs.widgets.meetings:Meetings",`
		`+ "hubs.widgets.about:About",`
		`+ "hubs.widgets.sticky:Sticky",`
		`+ "hubs.widgets.dummy:Dummy",`
		`])`

		`users = ['mrichard', 'duffy', 'ryanlerch', 'gnokii', 'nask0',`

abompard commented 7 years ago

This way, widgets can have a more complex configuration panel.

bowlofeggs commented on line 69 of hubs/tests/test_fedora_hubs_flask_api.py 7 years ago

I recommend either uncommenting or dropping this line.

bowlofeggs commented on line 31 of hubs/views/utils.py 7 years ago

I recommend adding a docblock here.

bowlofeggs commented on line 17 of hubs/views/utils.py 7 years ago

I recommend documenting the parameters in this docblock.

bowlofeggs commented on line 40 of hubs/views/utils.py 7 years ago

I recommend catching a more specific Exception type here, rather than catching any Exception. Perhaps catch the specific Exception that the validator would raise if this value were wrong (possibly a ValueError?)

bowlofeggs commented on line 47 of hubs/views/utils.py 7 years ago

I recommend filing a ticket about this.

bowlofeggs commented on line 54 of hubs/views/utils.py 7 years ago

I recommend logging this exception, otherwise it will be tough for an admin to debug the issue since the error message has been lost.

bowlofeggs commented on line 45 of hubs/views/widget.py 7 years ago

if widget_name in exclusive_widgets:
    flask.abort(402, 'Send bitcoins to abombard')

bowlofeggs commented on line 71 of hubs/views/widget.py 7 years ago

The above seems like a common pattern - you might consider making a decorator that accepts arguments (like ['right', 'left']) that can make sure the position is in the allowed list. Then you don't have to repeat this and decorators are fun to make ☺

bowlofeggs commented on line 71 of hubs/views/widget.py 7 years ago

Or you could just make it a function and call it, of course…

bowlofeggs commented on line 164 of hubs/views/widget.py 7 years ago

I recommend a ticket for this as well.

bowlofeggs commented on line 28 of hubs/widgets/base.py 7 years ago

You can add a Return: (or is it Returns:? I can't remember) block too that documents that a string is returned.

bowlofeggs commented on line 42 of hubs/widgets/base.py 7 years ago

You could add a Returns block here too.

bowlofeggs commented 7 years ago

My JavaScript/HTML/CSS skills are pretty weak, so I didn't have much to say about those portions of this PR. Everything else I wrote is a suggestion, though I would strongly recommend logging that one error I mentioned, as it will be frustrating for the user and the admin if that happens in the real world and there's no available debugging information.

LGTM.

abompard commented 7 years ago

Thanks for the review! I've implemented your suggestions (and will open the ticket for the TODO that I added).

2 new commits added