From 67775bc797e8cd04fbf38a5c94cb43513feb74a8 Mon Sep 17 00:00:00 2001 From: Ozzie Isaacs Date: Tue, 13 Apr 2021 19:08:02 +0200 Subject: [PATCH 1/4] Update requirements Catch error for invalid oauth tokens Fixes for displaying error messages on deleting books from list Fixes for displaying error messages on deleting bookformats --- cps/admin.py | 6 +- cps/editbooks.py | 19 +- cps/oauth_bb.py | 37 +- cps/templates/layout.html | 2 +- cps/web.py | 2 +- optional-requirements.txt | 2 +- test/Calibre-Web TestSummary_Linux.html | 567 +++++++++++++++++++----- 7 files changed, 514 insertions(+), 121 deletions(-) diff --git a/cps/admin.py b/cps/admin.py index 5cd31f18..d32d6d7c 100644 --- a/cps/admin.py +++ b/cps/admin.py @@ -245,7 +245,7 @@ def list_users(): off = int(request.args.get("offset") or 0) limit = int(request.args.get("limit") or 10) search = request.args.get("search") - sort = request.args.get("sort", "state") + sort = request.args.get("sort", "id") order = request.args.get("order", "").lower() state = None if sort == "state": @@ -254,7 +254,7 @@ def list_users(): if sort != "state" and order: order = text(sort + " " + order) elif not state: - order = ub.User.name.desc() + order = ub.User.id.asc() all_user = ub.session.query(ub.User) if not config.config_anonbrowse: @@ -371,7 +371,7 @@ def edit_list_user(param): 'message':_(u"No admin user remaining, can't remove admin role", nick=user.name)}), mimetype='application/json') user.role &= ~int(vals['field_index']) - elif param == 'sidebar_view': + elif param.startswith('sidebar'): if user.name == "Guest" and int(vals['field_index']) == constants.SIDEBAR_READ_AND_UNREAD: raise Exception(_("Guest can't have this view")) if vals['value'] == 'true': diff --git a/cps/editbooks.py b/cps/editbooks.py index 931ad13e..67ab06d9 100644 --- a/cps/editbooks.py +++ b/cps/editbooks.py @@ -315,19 +315,19 @@ def delete_book(book_id, book_format, jsonResponse): result, error = helper.delete_book(book, config.config_calibre_dir, book_format=book_format.upper()) if not result: if jsonResponse: - return json.dumps({"location": url_for("editbook.edit_book"), - "type": "alert", + return json.dumps([{"location": url_for("editbook.edit_book", book_id=book_id), + "type": "danger", "format": "", - "error": error}), + "message": error}]) else: flash(error, category="error") return redirect(url_for('editbook.edit_book', book_id=book_id)) if error: if jsonResponse: - warning = {"location": url_for("editbook.edit_book"), + warning = {"location": url_for("editbook.edit_book", book_id=book_id), "type": "warning", "format": "", - "error": error} + "message": error} else: flash(error, category="warning") if not book_format: @@ -339,6 +339,15 @@ def delete_book(book_id, book_format, jsonResponse): except Exception as ex: log.debug_or_exception(ex) calibre_db.session.rollback() + if jsonResponse: + return json.dumps([{"location": url_for("editbook.edit_book", book_id=book_id), + "type": "danger", + "format": "", + "message": ex}]) + else: + flash(str(ex), category="error") + return redirect(url_for('editbook.edit_book', book_id=book_id)) + else: # book not found log.error('Book with id "%s" could not be deleted: not found', book_id) diff --git a/cps/oauth_bb.py b/cps/oauth_bb.py index 5d909d91..c8cc2e3e 100644 --- a/cps/oauth_bb.py +++ b/cps/oauth_bb.py @@ -30,6 +30,7 @@ from flask_babel import gettext as _ from flask_dance.consumer import oauth_authorized, oauth_error from flask_dance.contrib.github import make_github_blueprint, github from flask_dance.contrib.google import make_google_blueprint, google +from oauthlib.oauth2 import TokenExpiredError, InvalidGrantError from flask_login import login_user, current_user, login_required from sqlalchemy.orm.exc import NoResultFound @@ -146,6 +147,7 @@ def bind_oauth_or_register(provider_id, provider_user_id, redirect_url, provider ub.session.add(oauth_entry) ub.session.commit() flash(_(u"Link to %(oauth)s Succeeded", oauth=provider_name), category="success") + log.info("Link to {} Succeeded".format(provider_name)) return redirect(url_for('web.profile')) except Exception as ex: log.debug_or_exception(ex) @@ -194,6 +196,7 @@ def unlink_oauth(provider): ub.session.commit() logout_oauth_user() flash(_(u"Unlink to %(oauth)s Succeeded", oauth=oauth_check[provider]), category="success") + log.info("Unlink to {} Succeeded".format(oauth_check[provider])) except Exception as ex: log.debug_or_exception(ex) ub.session.rollback() @@ -257,11 +260,13 @@ if ub.oauth_support: def github_logged_in(blueprint, token): if not token: flash(_(u"Failed to log in with GitHub."), category="error") + log.error("Failed to log in with GitHub") return False resp = blueprint.session.get("/user") if not resp.ok: flash(_(u"Failed to fetch user info from GitHub."), category="error") + log.error("Failed to fetch user info from GitHub") return False github_info = resp.json() @@ -273,11 +278,13 @@ if ub.oauth_support: def google_logged_in(blueprint, token): if not token: flash(_(u"Failed to log in with Google."), category="error") + log.error("Failed to log in with Google") return False resp = blueprint.session.get("/oauth2/v2/userinfo") if not resp.ok: flash(_(u"Failed to fetch user info from Google."), category="error") + log.error("Failed to fetch user info from Google") return False google_info = resp.json() @@ -318,11 +325,16 @@ if ub.oauth_support: def github_login(): if not github.authorized: return redirect(url_for('github.login')) - account_info = github.get('/user') - if account_info.ok: - account_info_json = account_info.json() - return bind_oauth_or_register(oauthblueprints[0]['id'], account_info_json['id'], 'github.login', 'github') - flash(_(u"GitHub Oauth error, please retry later."), category="error") + try: + account_info = github.get('/user') + if account_info.ok: + account_info_json = account_info.json() + return bind_oauth_or_register(oauthblueprints[0]['id'], account_info_json['id'], 'github.login', 'github') + flash(_(u"GitHub Oauth error, please retry later."), category="error") + log.error("GitHub Oauth error, please retry later") + except (InvalidGrantError, TokenExpiredError) as e: + flash(_(u"GitHub Oauth error: {}").format(e), category="error") + log.error(e) return redirect(url_for('web.login')) @@ -337,11 +349,16 @@ def github_login_unlink(): def google_login(): if not google.authorized: return redirect(url_for("google.login")) - resp = google.get("/oauth2/v2/userinfo") - if resp.ok: - account_info_json = resp.json() - return bind_oauth_or_register(oauthblueprints[1]['id'], account_info_json['id'], 'google.login', 'google') - flash(_(u"Google Oauth error, please retry later."), category="error") + try: + resp = google.get("/oauth2/v2/userinfo") + if resp.ok: + account_info_json = resp.json() + return bind_oauth_or_register(oauthblueprints[1]['id'], account_info_json['id'], 'google.login', 'google') + flash(_(u"Google Oauth error, please retry later."), category="error") + log.error("Google Oauth error, please retry later") + except (InvalidGrantError, TokenExpiredError) as e: + flash(_(u"Google Oauth error: {}").format(e), category="error") + log.error(e) return redirect(url_for('web.login')) diff --git a/cps/templates/layout.html b/cps/templates/layout.html index d4e0800e..6e68204c 100644 --- a/cps/templates/layout.html +++ b/cps/templates/layout.html @@ -92,7 +92,7 @@ {% for message in get_flashed_messages(with_categories=True) %} {%if message[0] == "error" %}
-
{{ message[1] }}
+
{{ message[1] }}
{%endif%} {%if message[0] == "info" %} diff --git a/cps/web.py b/cps/web.py index cf488986..65398e6c 100644 --- a/cps/web.py +++ b/cps/web.py @@ -756,7 +756,7 @@ def list_books(): off = int(request.args.get("offset") or 0) limit = int(request.args.get("limit") or config.config_books_per_page) search = request.args.get("search") - sort = request.args.get("sort", "state") + sort = request.args.get("sort", "id") order = request.args.get("order", "").lower() state = None diff --git a/optional-requirements.txt b/optional-requirements.txt index ca54fe4d..eb67f59b 100644 --- a/optional-requirements.txt +++ b/optional-requirements.txt @@ -26,7 +26,7 @@ python-ldap>=3.0.0,<3.4.0 Flask-SimpleLDAP>=1.4.0,<1.5.0 #oauth -Flask-Dance>=1.4.0,<3.1.0 +Flask-Dance>=1.4.0,<4.1.0 SQLAlchemy-Utils>=0.33.5,<0.38.0 # extracting metadata diff --git a/test/Calibre-Web TestSummary_Linux.html b/test/Calibre-Web TestSummary_Linux.html index 2d41ea3a..17df679b 100644 --- a/test/Calibre-Web TestSummary_Linux.html +++ b/test/Calibre-Web TestSummary_Linux.html @@ -37,20 +37,20 @@
-

Start Time: 2021-04-05 18:59:35

+

Start Time: 2021-04-12 21:44:07

-

Stop Time: 2021-04-05 21:34:25

+

Stop Time: 2021-04-13 00:22:44

-

Duration: 2h 5 min

+

Duration: 2h 7 min

@@ -1148,12 +1148,12 @@ - + TestEditBooksList 10 - 10 - 0 - 0 + 6 + 3 + 1 0 Detail @@ -1171,20 +1171,74 @@ - +
TestEditBooksList - test_bookslist_edit_categories
- PASS + +
+ ERROR +
+ + + + - +
TestEditBooksList - test_bookslist_edit_languages
- PASS + +
+ FAIL +
+ + + + @@ -1207,11 +1261,33 @@ - +
TestEditBooksList - test_bookslist_edit_seriesindex
- PASS + +
+ FAIL +
+ + + + @@ -1243,22 +1319,46 @@ - +
TestEditBooksList - test_search_books_list
- PASS + +
+ FAIL +
+ + + + - + TestEditBooksOnGdrive 20 18 - 2 - 0 + 1 + 1 0 Detail @@ -1293,12 +1393,13 @@ 
Traceback (most recent call last):
-  File "/home/ozzie/Development/calibre-web-test/test/test_edit_ebooks_gdrive.py", line 340, in test_edit_author
-    self.assertEqual(u'Sigurd Lindgren & Leo Baskerville', author.get_attribute('value'))
-AssertionError: 'Sigurd Lindgren & Leo Baskerville' != 'Sigurd Lindgren&Leo Baskerville'
-- Sigurd Lindgren & Leo Baskerville
-?                - -
-+ Sigurd Lindgren&Leo Baskerville
+ File "/home/ozzie/Development/calibre-web-test/test/test_edit_ebooks_gdrive.py", line 369, in test_edit_author + self.assertEqual(u'Pipo, Pipe', author.get_attribute('value')) +AssertionError: 'Pipo, Pipe' != 'Pipo| Pipe' +- Pipo, Pipe +? ^ ++ Pipo| Pipe +? ^
@@ -1425,11 +1526,31 @@ AssertionError: 'Sigurd Lindgren & Leo Baskerville' != 'Sigurd Lindgren&Leo Bask - +
TestEditBooksOnGdrive - test_edit_title
- PASS + +
+ ERROR +
+ + + + @@ -1461,31 +1582,11 @@ AssertionError: 'Sigurd Lindgren & Leo Baskerville' != 'Sigurd Lindgren&Leo Bask - +
TestEditBooksOnGdrive - test_watch_metadata
- -
- FAIL -
- - - - + PASS @@ -1701,13 +1802,13 @@ AssertionError: 'series' unexpectedly found in {'id': 5, 'reader': [], 'title': - + TestKoboSync 9 8 - 0 1 0 + 0 Detail @@ -1715,26 +1816,35 @@ AssertionError: 'series' unexpectedly found in {'id': 5, 'reader': [], 'title': - +
TestKoboSync - test_book_download
- ERROR + FAIL
- From f07cc8b10359ca83b426dc7ff316d3ea90c0e038 Mon Sep 17 00:00:00 2001 From: Ozzie Isaacs Date: Tue, 13 Apr 2021 19:26:10 +0200 Subject: [PATCH 2/4] Update optional-requirements: flask-dance Catch error for invalid oauth tokens Fixes for displaying error messages on deleting books from list Fixes for displaying error messages on deleting bookformats Removed non working sorting in books list --- cps/templates/book_table.html | 19 ++++++++++--------- cps/web.py | 1 - 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cps/templates/book_table.html b/cps/templates/book_table.html index 6a31c235..e700eb53 100644 --- a/cps/templates/book_table.html +++ b/cps/templates/book_table.html @@ -1,6 +1,7 @@ {% extends "layout.html" %} -{% macro text_table_row(parameter, edit_text, show_text, validate) -%} - {% endif %} - {{ text_table_row('title', _('Enter Title'),_('Title'), true) }} - {{ text_table_row('sort', _('Enter Title Sort'),_('Title Sort'), false) }} - {{ text_table_row('author_sort', _('Enter Author Sort'),_('Author Sort'), false) }} + {{ text_table_row('title', _('Enter Title'),_('Title'), true, true) }} + {{ text_table_row('sort', _('Enter Title Sort'),_('Title Sort'), false, true) }} + {{ text_table_row('author_sort', _('Enter Author Sort'),_('Author Sort'), false, true) }} {{ text_table_row('authors', _('Enter Authors'),_('Authors'), true) }} - {{ text_table_row('tags', _('Enter Categories'),_('Categories'), false) }} - {{ text_table_row('series', _('Enter Series'),_('Series'), false) }} + {{ text_table_row('tags', _('Enter Categories'),_('Categories'), false, false) }} + {{ text_table_row('series', _('Enter Series'),_('Series'), false, false) }} {{_('Series Index')}} - {{ text_table_row('languages', _('Enter Languages'),_('Languages'), false) }} + {{ text_table_row('languages', _('Enter Languages'),_('Languages'), false, false) }} - {{ text_table_row('publishers', _('Enter Publishers'),_('Publishers'), false) }} + {{ text_table_row('publishers', _('Enter Publishers'),_('Publishers'), false, false) }} {% if g.user.role_delete_books() and g.user.role_edit()%} {{_('Delete')}} {% endif %} diff --git a/cps/web.py b/cps/web.py index 65398e6c..52f2b82c 100644 --- a/cps/web.py +++ b/cps/web.py @@ -762,7 +762,6 @@ def list_books(): if sort == "state": state = json.loads(request.args.get("state", "[]")) - if sort != "state" and order: order = [text(sort + " " + order)] elif not state: From 0e1dbb5377aef12b4912ff43f30d554b0f770588 Mon Sep 17 00:00:00 2001 From: Ozzie Isaacs Date: Tue, 13 Apr 2021 19:41:44 +0200 Subject: [PATCH 3/4] Copy author names for displaying (#1935) --- cps/web.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cps/web.py b/cps/web.py index 52f2b82c..01c04350 100644 --- a/cps/web.py +++ b/cps/web.py @@ -26,6 +26,7 @@ from datetime import datetime import json import mimetypes import chardet # dependency of requests +import copy from babel.dates import format_date from babel import Locale as LC @@ -830,9 +831,10 @@ def author_list(): charlist = calibre_db.session.query(func.upper(func.substr(db.Authors.sort, 1, 1)).label('char')) \ .join(db.books_authors_link).join(db.Books).filter(calibre_db.common_filters()) \ .group_by(func.upper(func.substr(db.Authors.sort, 1, 1))).all() - for entry in entries: + autor_copy = copy.deepcopy(entries) + for entry in autor_copy: entry.Authors.name = entry.Authors.name.replace('|', ',') - return render_title_template('list.html', entries=entries, folder='web.books_list', charlist=charlist, + return render_title_template('list.html', entries=autor_copy, folder='web.books_list', charlist=charlist, title=u"Authors", page="authorlist", data='author', order=order_no) else: abort(404) From b38877e19361eb41f328642667d1f44170a7ed5d Mon Sep 17 00:00:00 2001 From: Ozzie Isaacs Date: Tue, 13 Apr 2021 19:47:55 +0200 Subject: [PATCH 4/4] Document change --- cps/web.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cps/web.py b/cps/web.py index 01c04350..0eaacdb1 100644 --- a/cps/web.py +++ b/cps/web.py @@ -831,6 +831,8 @@ def author_list(): charlist = calibre_db.session.query(func.upper(func.substr(db.Authors.sort, 1, 1)).label('char')) \ .join(db.books_authors_link).join(db.Books).filter(calibre_db.common_filters()) \ .group_by(func.upper(func.substr(db.Authors.sort, 1, 1))).all() + # If not creating a copy, readonly databases can not display authornames with "|" in it as changing the name + # starts a change session autor_copy = copy.deepcopy(entries) for entry in autor_copy: entry.Authors.name = entry.Authors.name.replace('|', ',')