From e6d4ab84da7a1031ab552b4a6b3fc4aa2348163e Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:41 +0000 Subject: [PATCH 1/9] [REF] web: move rpc_error binding into the crash_manager Moving the rpc_error binding into the crash manager is mandatory for it to be used in the website. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- .../src/js/chrome/abstract_web_client.js | 40 ------------------- .../static/src/js/services/crash_manager.js | 40 +++++++++++++++++++ 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/addons/web/static/src/js/chrome/abstract_web_client.js b/addons/web/static/src/js/chrome/abstract_web_client.js index 6b7df97b767..5adaaa36783 100644 --- a/addons/web/static/src/js/chrome/abstract_web_client.js +++ b/addons/web/static/src/js/chrome/abstract_web_client.js @@ -202,46 +202,6 @@ var AbstractWebClient = Widget.extend(ServiceProviderMixin, KeyboardNavigationMi }); core.bus.on('connection_lost', this, this._onConnectionLost); core.bus.on('connection_restored', this, this._onConnectionRestored); - - // crash manager integration - core.bus.on('rpc_error', crash_manager, crash_manager.rpc_error); - window.onerror = function (message, file, line, col, error) { - // Scripts injected in DOM (eg: google API's js files) won't return a clean error on window.onerror. - // The browser will just give you a 'Script error.' as message and nothing else for security issue. - // To enable onerror to work properly with CORS file, you should: - // 1. add crossorigin="anonymous" to your + @@ -247,7 +248,6 @@ - diff --git a/odoo/http.py b/odoo/http.py index 8702fd9a950..ab4512a6321 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -630,13 +630,18 @@ class JsonRequest(WebRequest): try: return super(JsonRequest, self)._handle_exception(exception) except Exception: - if not isinstance(exception, (odoo.exceptions.Warning, SessionExpiredException, - odoo.exceptions.except_orm, werkzeug.exceptions.NotFound)): - _logger.exception("Exception during JSON request handling.") + if not isinstance(exception, SessionExpiredException): + if exception.args and exception.args[0] == "bus.Bus not available in test mode": + _logger.info(exception) + elif isinstance(exception, (odoo.exceptions.Warning, odoo.exceptions.except_orm, + werkzeug.exceptions.NotFound)): + _logger.warning(exception) + else: + _logger.exception("Exception during JSON request handling.") error = { - 'code': 200, - 'message': "Odoo Server Error", - 'data': serialize_exception(exception) + 'code': 200, + 'message': "Odoo Server Error", + 'data': serialize_exception(exception), } if isinstance(exception, werkzeug.exceptions.NotFound): error['http_status'] = 404 @@ -1414,7 +1419,7 @@ class Root(object): httprequest = werkzeug.wrappers.Request(environ) httprequest.app = self httprequest.parameter_storage_class = werkzeug.datastructures.ImmutableOrderedMultiDict - + current_thread = threading.current_thread() current_thread.url = httprequest.url current_thread.query_count = 0 From dc561aae036e03dbec9d4085ce2baaa9286f530b Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:48 +0000 Subject: [PATCH 3/9] [IMP] website: enhance http error pages - The traceback is only visible if in debug mode or if edit mode. - All except_orm exceptions are now displayed as a 400 but the AccessError wich is a 403. There is now a dedicted page for the 400 error. - Errors messages are no longer the exception args but its name if the exception is an except_orm. Else there is no message since the current logic override exceptions of type Exception in ir_http thus we have no message to display. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- addons/website/models/ir_http.py | 7 +++ addons/website/views/website_templates.xml | 64 ++++++++++++++-------- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index e6eb9c10fa9..a00221596b4 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -214,6 +214,11 @@ class Http(models.AbstractModel): traceback=traceback.format_exc(), ) + # only except_orm exceptions contain a message + if isinstance(exception, odoo.exceptions.except_orm): + values['error_message'] = exception.name + code = 400 + if isinstance(exception, werkzeug.exceptions.HTTPException): if exception.code is None: # Hand-crafted HTTPException likely coming from abort(), @@ -279,6 +284,8 @@ class Http(models.AbstractModel): values['editable'] = request.uid and request.website.is_publisher() elif code == 403: logger.warn("403 Forbidden:\n\n%s", values['traceback']) + elif code == 400: + logger.warn("400 Bad Request:\n\n%s", values['traceback']) try: html = env['ir.ui.view'].render_template('website.%s' % view_id, values) except Exception: diff --git a/addons/website/views/website_templates.xml b/addons/website/views/website_templates.xml index aa264dd76a7..c7d1b9fb19b 100644 --- a/addons/website/views/website_templates.xml +++ b/addons/website/views/website_templates.xml @@ -911,19 +911,22 @@ + + + + @@ -982,7 +1004,7 @@
-

404: Page not found!

+

404: Page not found!

The page you were looking for could not be found; it is possible you have typed the address incorrectly, but it has most probably been removed due @@ -995,10 +1017,6 @@

- - - - From 83c1d56d2f3d22672803118f33c2f702721df1e5 Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:50 +0000 Subject: [PATCH 4/9] [REF] mail, website, *: remove useless "in" classes * website_sale_comparison Just removing old "in" class from bootstrap3 wich has been replaced by "show" in bootstrap4. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- addons/mail/static/src/xml/activity.xml | 2 +- addons/website/views/website_templates.xml | 2 +- .../views/website_sale_comparison_template.xml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/mail/static/src/xml/activity.xml b/addons/mail/static/src/xml/activity.xml index 217e1a39f5c..11b4b0e9022 100644 --- a/addons/mail/static/src/xml/activity.xml +++ b/addons/mail/static/src/xml/activity.xml @@ -12,7 +12,7 @@ -
+
diff --git a/addons/website/views/website_templates.xml b/addons/website/views/website_templates.xml index c7d1b9fb19b..c76c381d2a2 100644 --- a/addons/website/views/website_templates.xml +++ b/addons/website/views/website_templates.xml @@ -924,7 +924,7 @@

Error

-
+
diff --git a/addons/website_sale_comparison/views/website_sale_comparison_template.xml b/addons/website_sale_comparison/views/website_sale_comparison_template.xml index 1e74c237e3e..06cf34d6b64 100644 --- a/addons/website_sale_comparison/views/website_sale_comparison_template.xml +++ b/addons/website_sale_comparison/views/website_sale_comparison_template.xml @@ -46,7 +46,7 @@ - + or @@ -126,7 +126,7 @@ - + From 90818fd083774988c331d741d34181a5af5ecd3a Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:53 +0000 Subject: [PATCH 5/9] [IMP] test_website: add test tour for the website errors The purpose of these tests is to verify that errors informations are displayed correctly on the website. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- addons/test_website/controllers/main.py | 61 ++++++- .../test_website/data/test_website_data.xml | 38 +++++ .../test_website/static/src/js/test_error.js | 30 ++++ .../static/tests/tours/error_views.js | 152 ++++++++++++++++++ addons/test_website/tests/__init__.py | 1 + addons/test_website/tests/test_error.py | 10 ++ addons/test_website/views/templates.xml | 9 +- 7 files changed, 297 insertions(+), 4 deletions(-) create mode 100644 addons/test_website/static/src/js/test_error.js create mode 100644 addons/test_website/static/tests/tours/error_views.js create mode 100644 addons/test_website/tests/test_error.py diff --git a/addons/test_website/controllers/main.py b/addons/test_website/controllers/main.py index ae45d160b06..cc60ccfd25f 100644 --- a/addons/test_website/controllers/main.py +++ b/addons/test_website/controllers/main.py @@ -1,16 +1,19 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import json +import werkzeug + from odoo import http from odoo.http import request from odoo.addons.portal.controllers.web import Home -import json +from odoo.exceptions import UserError, ValidationError, AccessError, MissingError, AccessError, AccessDenied class WebsiteTest(Home): - @http.route('/test_view', type='http', auth="public", website=True) - def test_view(self, **kw): + @http.route('/test_view', type='http', auth='public', website=True) + def test_view(self, **kwargs): return request.render('test_website.test_view') @http.route('/ignore_args/converteronly//', type='http', auth="public", website=True) @@ -40,3 +43,55 @@ class WebsiteTest(Home): @http.route('/multi_company_website', type='http', auth="public", website=True) def test_company_context(self): return request.make_response(json.dumps(request.context.get('allowed_company_ids'))) + + @http.route('/test_error_view', type='http', auth='public', website=True) + def test_error_view(self, **kwargs): + return request.render('test_website.test_error_view') + + @http.route('/test_user_error_http', type='http', auth='public', website=True) + def test_user_error_http(self, **kwargs): + raise UserError("This is a user http test") + + @http.route('/test_user_error_json', type='json', auth='public', website=True) + def test_user_error_json(self, **kwargs): + raise UserError("This is a user rpc test") + + @http.route('/test_validation_error_http', type='http', auth='public', website=True) + def test_validation_error_http(self, **kwargs): + raise ValidationError("This is a validation http test") + + @http.route('/test_validation_error_json', type='json', auth='public', website=True) + def test_validation_error_json(self, **kwargs): + raise ValidationError("This is a validation rpc test") + + @http.route('/test_access_error_json', type='json', auth='public', website=True) + def test_access_error_json(self, **kwargs): + raise AccessError("This is an access rpc test") + + @http.route('/test_access_error_http', type='http', auth='public', website=True) + def test_access_error_http(self, **kwargs): + raise AccessError("This is an access http test") + + @http.route('/test_missing_error_json', type='json', auth='public', website=True) + def test_missing_error_json(self, **kwargs): + raise MissingError("This is a missing rpc test") + + @http.route('/test_missing_error_http', type='http', auth='public', website=True) + def test_missing_error_http(self, **kwargs): + raise MissingError("This is a missing http test") + + @http.route('/test_internal_error_json', type='json', auth='public', website=True) + def test_internal_error_json(self, **kwargs): + raise werkzeug.exceptions.InternalServerError() + + @http.route('/test_internal_error_http', type='http', auth='public', website=True) + def test_internal_error_http(self, **kwargs): + raise werkzeug.exceptions.InternalServerError() + + @http.route('/test_access_denied_json', type='json', auth='public', website=True) + def test_denied_error_json(self, **kwargs): + raise AccessDenied("This is an access denied rpc test") + + @http.route('/test_access_denied_http', type='http', auth='public', website=True) + def test_denied_error_http(self, **kwargs): + raise AccessDenied("This is an access denied http test") diff --git a/addons/test_website/data/test_website_data.xml b/addons/test_website/data/test_website_data.xml index 0d829aaab84..05a248190d4 100644 --- a/addons/test_website/data/test_website_data.xml +++ b/addons/test_website/data/test_website_data.xml @@ -30,6 +30,44 @@ + + Test Error View + qweb + test_website.test_error_view + + + +
+

Test Error View

+
+ + +
+
+ + + + True /test_page_view diff --git a/addons/test_website/static/src/js/test_error.js b/addons/test_website/static/src/js/test_error.js new file mode 100644 index 00000000000..8a0f909d33f --- /dev/null +++ b/addons/test_website/static/src/js/test_error.js @@ -0,0 +1,30 @@ +odoo.define('website_forum.test_error', function (require) { +'use strict'; + +var publicWidget = require('web.public.widget'); + +publicWidget.registry.testError = publicWidget.Widget.extend({ + selector: '.rpc_error', + events: { + 'click a': '_onRpcErrorClick', + }, + + //---------------------------------------------------------------------- + // Handlers + //---------------------------------------------------------------------- + + /** + * make a rpc call with the href of the DOM element clicked + * @private + * @param {Event} ev + * @returns {Promise} + */ + _onRpcErrorClick: function (ev) { + ev.preventDefault(); + var $link = $(ev.currentTarget); + return this._rpc({ + route: $link.attr('href'), + }); + } +}); +}); diff --git a/addons/test_website/static/tests/tours/error_views.js b/addons/test_website/static/tests/tours/error_views.js new file mode 100644 index 00000000000..a3d9f3fa035 --- /dev/null +++ b/addons/test_website/static/tests/tours/error_views.js @@ -0,0 +1,152 @@ +odoo.define('test_website.error_views', function (require) { +'use strict'; + +var tour = require('web_tour.tour'); + +tour.register('test_error_website', { + test: true, + url: '/test_error_view', +}, +[ + // RPC ERROR + { + content: "trigger rpc user error", + trigger: 'a[href="/test_user_error_json"]', + }, { + content: "rpc user error modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a user rpc test")', + trigger: 'button.btn.btn-primary', + }, { + content: "trigger rpc access error", + trigger: 'a[href="/test_access_error_json"]', + }, { + content: "rpc access error modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is an access rpc test")', + trigger: 'button.btn.btn-primary', + }, { + content: "trigger validation rpc error", + trigger: 'a[href="/test_validation_error_json"]', + }, { + content: "rpc validation error modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a validation rpc test")', + trigger: 'button.btn.btn-primary', + }, { + content: "trigger rpc missing error", + trigger: 'a[href="/test_missing_error_json"]', + }, { + content: "rpc missing error modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a missing rpc test")', + trigger: 'button.btn.btn-primary', + }, { + content: "trigger rpc error 403", + trigger: 'a[href="/test_access_denied_json"]', + }, { + content: "rpc error 403 modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is an access denied rpc test")', + trigger: 'button.btn.btn-primary', + }, { + content: "trigger rpc error 500", + trigger: 'a[href="/test_internal_error_json"]', + }, { + content: "rpc error 500 modal is an ErrorDialog", + extra_trigger: 'div.o_dialog_error.modal-body div.alert.alert-warning', + trigger: 'button.btn.btn-primary[type="button"]', + }, + // HTTP ERROR + { + content: "trigger http user error", + trigger: 'body', + run: function () { + window.location.href = window.location.origin + '/test_user_error_http?debug=0'; + }, + }, { + content: "http user error page has title and message", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div.container pre:contains("This is a user http test")', + run: function () { + window.location.href = window.location.origin + '/test_user_error_http?debug=1'; + }, + }, { + content: "http user error page debug has title and message open", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div#error_main.collapse.show pre:contains("This is a user http test")', + run: function () {}, + }, { + content: "http user error page debug has traceback closed", + trigger: 'body:has(div#error_traceback.collapse:not(.show) pre#exception_traceback)', + run: function () { + window.location.href = window.location.origin + '/test_validation_error_http?debug=0'; + }, + }, { + content: "http validation error page has title and message", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div.container pre:contains("This is a validation http test")', + run: function () { + window.location.href = window.location.origin + '/test_validation_error_http?debug=1'; + }, + }, { + content: "http validation error page debug has title and message open", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div#error_main.collapse.show pre:contains("This is a validation http test")', + run: function () {}, + }, { + content: "http validation error page debug has traceback closed", + trigger: 'body:has(div#error_traceback.collapse:not(.show) pre#exception_traceback)', + run: function () { + window.location.href = window.location.origin + '/test_access_error_http?debug=0'; + }, + }, { + content: "http access error page has title and message", + extra_trigger: 'h1:contains("403: Forbidden")', + trigger: 'div.container pre:contains("This is an access http test")', + run: function () { + window.location.href = window.location.origin + '/test_access_error_http?debug=1'; + }, + }, { + content: "http access error page debug has title and message open", + extra_trigger: 'h1:contains("403: Forbidden")', + trigger: 'div#error_main.collapse.show pre:contains("This is an access http test")', + run: function () {}, + }, { + content: "http access error page debug has traceback closed", + trigger: 'body:has(div#error_traceback.collapse:not(.show) pre#exception_traceback)', + run: function () { + window.location.href = window.location.origin + '/test_missing_error_http?debug=0'; + }, + }, { + content: "http missing error page has title and message", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div.container pre:contains("This is a missing http test")', + run: function () { + window.location.href = window.location.origin + '/test_missing_error_http?debug=1'; + }, + }, { + content: "http missing error page debug has title and message open", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div#error_main.collapse.show pre:contains("This is a missing http test")', + run: function () {}, + }, { + content: "http missing error page debug has traceback closed", + trigger: 'body:has(div#error_traceback.collapse:not(.show) pre#exception_traceback)', + run: function () { + window.location.href = window.location.origin + '/test_karma_error_http?debug=0'; + }, + }, { + content: "http error 403 page has title but no message", + extra_trigger: 'h1:contains("403: Forbidden")', + trigger: 'div#wrap:not(:has(pre:contains("This is an access denied http test"))', //See ir_http.py handle_exception, the exception is replaced so there is no message ! + run: function () { + window.location.href = window.location.origin + '/test_access_denied_http?debug=1'; + }, + }, { + content: "http 403 error page debug has title but no message", + extra_trigger: 'h1:contains("403: Forbidden")', + trigger: 'div#debug_infos:not(:has(#error_main))', + run: function () {}, + }, { + content: "http 403 error page debug has traceback open", + trigger: 'body:has(div#error_traceback.collapse.show pre#exception_traceback)', + run: function () {}, + }, +]); +}); diff --git a/addons/test_website/tests/__init__.py b/addons/test_website/tests/__init__.py index 4bb004cd8f9..8b8ec3e4e44 100644 --- a/addons/test_website/tests/__init__.py +++ b/addons/test_website/tests/__init__.py @@ -3,5 +3,6 @@ from . import test_controller_args from . import test_reset_views +from . import test_error from . import test_views_during_module_operation from . import test_multi_company diff --git a/addons/test_website/tests/test_error.py b/addons/test_website/tests/test_error.py new file mode 100644 index 00000000000..63dad4133f1 --- /dev/null +++ b/addons/test_website/tests/test_error.py @@ -0,0 +1,10 @@ +import odoo.tests +from odoo.tools import mute_logger + + +@odoo.tests.common.tagged('post_install', '-at_install') +class TestWebsiteError(odoo.tests.HttpCase): + + @mute_logger('odoo.addons.website.models.ir_http', 'odoo.http') + def test_01_run_test(self): + self.start_tour("/test_error_view", 'test_error_website') diff --git a/addons/test_website/views/templates.xml b/addons/test_website/views/templates.xml index 4a6389cf9c7..ee6b500c5ca 100644 --- a/addons/test_website/views/templates.xml +++ b/addons/test_website/views/templates.xml @@ -1,8 +1,15 @@ + + From 0d1407a715901ea06e9a7211c0e3dbe09fadb785 Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:55 +0000 Subject: [PATCH 6/9] [IMP] base, web, *: make KarmaError an except_orm * = gamification, test_website, stock - KarmaError is now handled as a 400 exception. - test_website has been updated. - NO_POSTMORTEM is now clean it was referencing duplicates as most the exceptions inherit from except_orm. - serialize_exception from http.py has been moved to ir_http to take advantage of the odoo inheritance system. We can then extend ir_http serialize_exception method to add the KarmaError logic if and only if gamification is installed. Places where serialize_exception was previously used are updated. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- .../models/gamification_karma_rank.py | 21 ++++++++--- addons/stock/controllers/main.py | 3 +- addons/test_website/__manifest__.py | 1 + addons/test_website/controllers/main.py | 9 +++++ .../static/tests/tours/error_views.js | 25 +++++++++++++ addons/web/controllers/main.py | 7 ++-- .../static/src/js/services/crash_manager.js | 1 + odoo/addons/base/models/ir_http.py | 30 ++++++++++++++- odoo/http.py | 37 +++---------------- 9 files changed, 90 insertions(+), 44 deletions(-) diff --git a/addons/gamification/models/gamification_karma_rank.py b/addons/gamification/models/gamification_karma_rank.py index 78b8e40ac50..cd4ef6cc28a 100644 --- a/addons/gamification/models/gamification_karma_rank.py +++ b/addons/gamification/models/gamification_karma_rank.py @@ -1,15 +1,24 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. - -from werkzeug.exceptions import Forbidden - -from odoo import api, tools, fields, models +from odoo import api, tools, fields, models, exceptions, http from odoo.tools.translate import html_translate -class KarmaError(Forbidden): +class KarmaError(exceptions.except_orm): """ Karma-related error, used for forum and posts. """ - pass + def __init__(self, msg): + super(KarmaError, self).__init__(msg) + + +class Http(models.AbstractModel): + _inherit = 'ir.http' + + @classmethod + def serialize_exception(self, e): + res = super(Http, self).serialize_exception(e) + if isinstance(e, KarmaError): + res["exception_type"] = "karma_error" + return res class KarmaRank(models.Model): diff --git a/addons/stock/controllers/main.py b/addons/stock/controllers/main.py index e18b0e6da96..05d6f5b0ead 100644 --- a/addons/stock/controllers/main.py +++ b/addons/stock/controllers/main.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- from odoo import http from odoo.http import request -from odoo.addons.web.controllers.main import _serialize_exception from odoo.tools import html_escape import json @@ -37,7 +36,7 @@ class StockReportController(http.Controller): response.set_cookie('fileToken', token) return response except Exception as e: - se = _serialize_exception(e) + se = request.env['ir.http'].serialize_exception(e) error = { 'code': 200, 'message': 'Odoo Server Error', diff --git a/addons/test_website/__manifest__.py b/addons/test_website/__manifest__.py index 557722971d4..e1bac638412 100644 --- a/addons/test_website/__manifest__.py +++ b/addons/test_website/__manifest__.py @@ -14,6 +14,7 @@ dependencies. Neither we want to add in website module some routes, views and models which only purpose is to run tests.""", 'depends': [ 'website', + 'gamification', ], 'data': [ 'views/templates.xml', diff --git a/addons/test_website/controllers/main.py b/addons/test_website/controllers/main.py index cc60ccfd25f..0a8210881c7 100644 --- a/addons/test_website/controllers/main.py +++ b/addons/test_website/controllers/main.py @@ -8,6 +8,7 @@ from odoo import http from odoo.http import request from odoo.addons.portal.controllers.web import Home from odoo.exceptions import UserError, ValidationError, AccessError, MissingError, AccessError, AccessDenied +from odoo.addons.gamification.models.gamification_karma_rank import KarmaError class WebsiteTest(Home): @@ -80,6 +81,14 @@ class WebsiteTest(Home): def test_missing_error_http(self, **kwargs): raise MissingError("This is a missing http test") + @http.route('/test_karma_error_json', type='json', auth='public', website=True) + def test_karma_error_rpc(self, **kwargs): + raise KarmaError("This is a karma rpc test") + + @http.route('/test_karma_error_http', type='http', auth='public', website=True) + def test_karma_error_http(self, **kwargs): + raise KarmaError("This is a karma http test") + @http.route('/test_internal_error_json', type='json', auth='public', website=True) def test_internal_error_json(self, **kwargs): raise werkzeug.exceptions.InternalServerError() diff --git a/addons/test_website/static/tests/tours/error_views.js b/addons/test_website/static/tests/tours/error_views.js index a3d9f3fa035..f76ba28ff8e 100644 --- a/addons/test_website/static/tests/tours/error_views.js +++ b/addons/test_website/static/tests/tours/error_views.js @@ -44,6 +44,13 @@ tour.register('test_error_website', { content: "rpc error 403 modal has message", extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is an access denied rpc test")', trigger: 'button.btn.btn-primary', + }, { + content: "trigger rpc missing error", + trigger: 'a[href="/test_karma_error_json"]', + }, { + content: "rpc missing error modal has message", + extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a karma rpc test")', + trigger: 'button.btn.btn-primary', }, { content: "trigger rpc error 500", trigger: 'a[href="/test_internal_error_json"]', @@ -131,6 +138,24 @@ tour.register('test_error_website', { run: function () { window.location.href = window.location.origin + '/test_karma_error_http?debug=0'; }, + }, { + content: "http karma error page has title and message", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div.container pre:contains("This is a karma http test")', + run: function () { + window.location.href = window.location.origin + '/test_karma_error_http?debug=1'; + }, + }, { + content: "http karma error page debug has title and message open", + extra_trigger: 'h1:contains("Oops! Something went wrong.")', + trigger: 'div#error_main.collapse.show pre:contains("This is a karma http test")', + run: function () {}, + }, { + content: "http karma error page debug has traceback closed", + trigger: 'body:has(div#error_traceback.collapse:not(.show) pre#exception_traceback)', + run: function () { + window.location.href = window.location.origin + '/test_access_denied_http?debug=0'; + }, }, { content: "http error 403 page has title but no message", extra_trigger: 'h1:contains("403: Forbidden")', diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index 860cf454dd4..7aca92712a8 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -42,8 +42,7 @@ from odoo.tools.translate import _ from odoo.tools.misc import str2bool, xlwt, file_open from odoo.tools.safe_eval import safe_eval from odoo import http, tools -from odoo.http import content_disposition, dispatch_rpc, request, \ - serialize_exception as _serialize_exception, Response +from odoo.http import content_disposition, dispatch_rpc, request, Response from odoo.exceptions import AccessError, UserError, AccessDenied from odoo.models import check_method_name from odoo.service import db, security @@ -83,7 +82,7 @@ def serialize_exception(f): return f(*args, **kwargs) except Exception as e: _logger.exception("An exception occured during an http request") - se = _serialize_exception(e) + se = request.registry['ir.http'].serialize_exception(e) error = { 'code': 200, 'message': "Odoo Server Error", @@ -1694,7 +1693,7 @@ class ReportController(http.Controller): else: return except Exception as e: - se = _serialize_exception(e) + se = request.env['ir.http'].serialize_exception(e) error = { 'code': 200, 'message': "Odoo Server Error", diff --git a/addons/web/static/src/js/services/crash_manager.js b/addons/web/static/src/js/services/crash_manager.js index 8681a869aba..a8fafa4b897 100644 --- a/addons/web/static/src/js/services/crash_manager.js +++ b/addons/web/static/src/js/services/crash_manager.js @@ -17,6 +17,7 @@ var map_title ={ validation_error: _lt("Validation Error"), except_orm: _lt("Global Business Error"), access_denied: _lt("Access Denied"), + karma_error: _lt("Karma Error"), }; /** diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 897ca1de9ba..3f49b4052fc 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/odoo/addons/base/models/ir_http.py @@ -9,6 +9,7 @@ import mimetypes import os import re import sys +import traceback import werkzeug import werkzeug.exceptions @@ -20,7 +21,7 @@ import odoo from odoo import api, http, models, tools, SUPERUSER_ID from odoo.exceptions import AccessDenied, AccessError from odoo.http import request, STATIC_CACHE, content_disposition -from odoo.tools import pycompat, consteq +from odoo.tools import consteq, pycompat, ustr from odoo.tools.mimetypes import guess_mimetype from odoo.modules.module import get_resource_path, get_module_path @@ -178,6 +179,33 @@ class IrHttp(models.AbstractModel): return attach return False + @classmethod + def serialize_exception(self, e): + tmp = { + "name": type(e).__module__ + "." + type(e).__name__ if type(e).__module__ else type(e).__name__, + "debug": traceback.format_exc(), + "message": ustr(e), + "arguments": e.args, + "exception_type": "internal_error" + } + if isinstance(e, odoo.exceptions.UserError): + tmp["exception_type"] = "user_error" + elif isinstance(e, odoo.exceptions.Warning): + tmp["exception_type"] = "warning" + elif isinstance(e, odoo.exceptions.RedirectWarning): + tmp["exception_type"] = "warning" + elif isinstance(e, odoo.exceptions.AccessError): + tmp["exception_type"] = "access_error" + elif isinstance(e, odoo.exceptions.MissingError): + tmp["exception_type"] = "missing_error" + elif isinstance(e, odoo.exceptions.AccessDenied): + tmp["exception_type"] = "access_denied" + elif isinstance(e, odoo.exceptions.ValidationError): + tmp["exception_type"] = "validation_error" + elif isinstance(e, odoo.exceptions.except_orm): + tmp["exception_type"] = "except_orm" + return tmp + @classmethod def _handle_exception(cls, exception): # in case of Exception, e.g. 404, we don't step into _dispatch diff --git a/odoo/http.py b/odoo/http.py index ab4512a6321..25200ebcecf 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -98,16 +98,16 @@ def replace_request_password(args): args[2] = '*' return tuple(args) + # don't trigger debugger for those exceptions, they carry user-facing warnings # and indications, they're not necessarily indicative of anything being # *broken* -NO_POSTMORTEM = (odoo.osv.orm.except_orm, - odoo.exceptions.AccessError, - odoo.exceptions.ValidationError, - odoo.exceptions.MissingError, +NO_POSTMORTEM = (odoo.exceptions.except_orm, odoo.exceptions.AccessDenied, odoo.exceptions.Warning, odoo.exceptions.RedirectWarning) + + def dispatch_rpc(service_name, method, params): """ Handle a RPC call. @@ -316,7 +316,7 @@ class WebRequest(object): """Called within an except block to allow converting exceptions to abitrary responses. Anything returned (except None) will be used as response.""" - self._failed = exception # prevent tx commit + self._failed = exception # prevent tx commit if not isinstance(exception, NO_POSTMORTEM) \ and not isinstance(exception, werkzeug.exceptions.HTTPException): odoo.tools.debugger.post_mortem( @@ -641,7 +641,7 @@ class JsonRequest(WebRequest): error = { 'code': 200, 'message': "Odoo Server Error", - 'data': serialize_exception(exception), + 'data': request.registry['ir.http'].serialize_exception(exception), } if isinstance(exception, werkzeug.exceptions.NotFound): error['http_status'] = 404 @@ -691,31 +691,6 @@ class JsonRequest(WebRequest): except Exception as e: return self._handle_exception(e) -def serialize_exception(e): - tmp = { - "name": type(e).__module__ + "." + type(e).__name__ if type(e).__module__ else type(e).__name__, - "debug": traceback.format_exc(), - "message": ustr(e), - "arguments": e.args, - "exception_type": "internal_error" - } - if isinstance(e, odoo.exceptions.UserError): - tmp["exception_type"] = "user_error" - elif isinstance(e, odoo.exceptions.Warning): - tmp["exception_type"] = "warning" - elif isinstance(e, odoo.exceptions.RedirectWarning): - tmp["exception_type"] = "warning" - elif isinstance(e, odoo.exceptions.AccessError): - tmp["exception_type"] = "access_error" - elif isinstance(e, odoo.exceptions.MissingError): - tmp["exception_type"] = "missing_error" - elif isinstance(e, odoo.exceptions.AccessDenied): - tmp["exception_type"] = "access_denied" - elif isinstance(e, odoo.exceptions.ValidationError): - tmp["exception_type"] = "validation_error" - elif isinstance(e, odoo.exceptions.except_orm): - tmp["exception_type"] = "except_orm" - return tmp class HttpRequest(WebRequest): """ Handler for the ``http`` request type. From 283f8fed6e85f2df0386de46138129b4cadc52d7 Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:26:57 +0000 Subject: [PATCH 7/9] [IMP] web, *: display rpc warnings with toasts * = test_website, partner_autocomplete - All the rpc warnings are now displayed using the notification system of https://github.com/odoo/odoo/commit/b94395b50175925fc7d1bd3655e5facb78a8ba2b - test_website is up to date Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- .../tests/partner_autocomplete_tests.js | 8 +++- .../static/tests/tours/error_views.js | 24 +++++------ .../src/js/chrome/abstract_web_client.js | 5 ++- .../web/static/src/js/core/service_mixins.js | 40 +++++++++++-------- .../static/src/js/services/crash_manager.js | 27 ++++++++----- .../src/js/services/notification_service.js | 1 + .../web/static/src/js/widgets/notification.js | 2 + addons/web/static/src/xml/notification.xml | 4 +- 8 files changed, 68 insertions(+), 43 deletions(-) diff --git a/addons/partner_autocomplete/static/tests/partner_autocomplete_tests.js b/addons/partner_autocomplete/static/tests/partner_autocomplete_tests.js index ca1144b8822..b524dba75a6 100644 --- a/addons/partner_autocomplete/static/tests/partner_autocomplete_tests.js +++ b/addons/partner_autocomplete/static/tests/partner_autocomplete_tests.js @@ -81,7 +81,13 @@ odoo.define('partner_autocomplete.tests', function (require) { return this._getOdooSuggestions(value); }, do_notify: function (title, message, sticky, className) { - return this.displayNotification(title, message, 'warning', true, 'o_partner_autocomplete_test_notify'); + return this.displayNotification({ + type: 'warning', + title: title, + message: message, + sticky: sticky, + className: 'o_partner_autocomplete_test_notify' + }); }, }); }); diff --git a/addons/test_website/static/tests/tours/error_views.js b/addons/test_website/static/tests/tours/error_views.js index f76ba28ff8e..5983d514a82 100644 --- a/addons/test_website/static/tests/tours/error_views.js +++ b/addons/test_website/static/tests/tours/error_views.js @@ -14,43 +14,43 @@ tour.register('test_error_website', { trigger: 'a[href="/test_user_error_json"]', }, { content: "rpc user error modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a user rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is a user rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger rpc access error", trigger: 'a[href="/test_access_error_json"]', }, { content: "rpc access error modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is an access rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is an access rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger validation rpc error", trigger: 'a[href="/test_validation_error_json"]', }, { content: "rpc validation error modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a validation rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is a validation rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger rpc missing error", trigger: 'a[href="/test_missing_error_json"]', }, { content: "rpc missing error modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a missing rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is a missing rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger rpc error 403", trigger: 'a[href="/test_access_denied_json"]', }, { content: "rpc error 403 modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is an access denied rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is an access denied rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger rpc missing error", trigger: 'a[href="/test_karma_error_json"]', }, { content: "rpc missing error modal has message", - extra_trigger: 'div.o_dialog_warning.modal-body:contains("This is a karma rpc test")', - trigger: 'button.btn.btn-primary', + extra_trigger: 'div.toast-body:contains("This is a karma rpc test")', + trigger: 'button.o_notification_close', }, { content: "trigger rpc error 500", trigger: 'a[href="/test_internal_error_json"]', diff --git a/addons/web/static/src/js/chrome/abstract_web_client.js b/addons/web/static/src/js/chrome/abstract_web_client.js index 270f2871afe..2d94501deed 100644 --- a/addons/web/static/src/js/chrome/abstract_web_client.js +++ b/addons/web/static/src/js/chrome/abstract_web_client.js @@ -331,7 +331,7 @@ var AbstractWebClient = Widget.extend(ServiceProviderMixin, KeyboardNavigationMi * @private */ _onConnectionLost: function () { - this.connectionNotificationID = this.call('notification', 'notify', { + this.connectionNotificationID = this.displayNotification({ title: _t('Connection lost'), message: _t('Trying to reconnect...'), sticky: true @@ -345,7 +345,8 @@ var AbstractWebClient = Widget.extend(ServiceProviderMixin, KeyboardNavigationMi _onConnectionRestored: function () { if (this.connectionNotificationID) { this.call('notification', 'close', this.connectionNotificationID); - this.call('notification', 'notify', { + this.displayNotification({ + type: 'info', title: _t('Connection restored'), message: _t('You are back online'), sticky: false diff --git a/addons/web/static/src/js/core/service_mixins.js b/addons/web/static/src/js/core/service_mixins.js index 209f6ccabb3..f34adfe4de8 100644 --- a/addons/web/static/src/js/core/service_mixins.js +++ b/addons/web/static/src/js/core/service_mixins.js @@ -225,34 +225,42 @@ var ServicesMixin = { /** * Displays a notification. * - * @param {string} title - * @param {string} [message] - * @param {string} [type='warning'] 'info', 'success', 'warning', 'danger' or '' - * @param {boolean} [sticky=false] - * @param {string} [className] + * @param {Object} options + * @param {string} options.title + * @param {string} [options.subtitle] + * @param {string} [options.message] + * @param {string} [options.type='warning'] 'info', 'success', 'warning', 'danger' or '' + * @param {boolean} [options.sticky=false] + * @param {string} [options.className] */ - displayNotification: function (title, message, type, sticky, className) { - return this.call('notification', 'notify', { - type: type, + displayNotification: function (options) { + return this.call('notification', 'notify', options); + }, + /** + * @deprecated will be removed as soon as the notification system is reviewed + * @see displayNotification + */ + do_notify: function (title, message, sticky, className) { + return this.displayNotification({ + type: 'warning', title: title, message: message, sticky: sticky, className: className, }); }, - /** - * @deprecated will be removed as soon as the notification system is reviewed - * @see displayNotification - */ - do_notify: function (title, message, sticky, className) { - return this.displayNotification(title, message, 'warning', sticky, className); - }, /** * @deprecated will be removed as soon as the notification system is reviewed * @see displayNotification */ do_warn: function (title, message, sticky, className) { - return this.displayNotification(title, message, 'danger', sticky, className); + return this.displayNotification({ + type: 'danger', + title: title, + message: message, + sticky: sticky, + className: className, + }); }, }; diff --git a/addons/web/static/src/js/services/crash_manager.js b/addons/web/static/src/js/services/crash_manager.js index a8fafa4b897..6a78d776afa 100644 --- a/addons/web/static/src/js/services/crash_manager.js +++ b/addons/web/static/src/js/services/crash_manager.js @@ -4,6 +4,9 @@ odoo.define('web.CrashManager', function (require) { var ajax = require('web.ajax'); var core = require('web.core'); var Dialog = require('web.Dialog'); +var mixins = require('web.mixins'); +var ServicesMixin = require('web.ServicesMixin'); +var ServiceProviderMixin = require('web.ServiceProviderMixin'); var Widget = require('web.Widget'); var _t = core._t; @@ -75,12 +78,14 @@ var WarningDialog = CrashManagerDialog.extend({ }, }); -var CrashManager = core.Class.extend({ +var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, ServiceProviderMixin,{ init: function () { var self = this; this.active = true; this.isConnected = true; + mixins.EventDispatcherMixin.init.call(this); + // crash manager integration core.bus.on('rpc_error', this, this.rpc_error); window.onerror = function (message, file, line, col, error) { @@ -139,6 +144,7 @@ var CrashManager = core.Class.extend({ ev.preventDefault(); } }); + ServiceProviderMixin.init.call(this); }, enable: function () { this.active = true; @@ -192,7 +198,7 @@ var CrashManager = core.Class.extend({ data: _.extend({}, error.data, { message: error.data.arguments[1], - title: error.data.arguments[0] !== 'Warning' ? (" - " + error.data.arguments[0]) : '', + title: error.data.arguments[0] !== 'Warning' ? error.data.arguments[0] : '', }) }); } @@ -213,14 +219,11 @@ var CrashManager = core.Class.extend({ data: _.extend({}, error.data, { message: error.data.arguments[0], - title: map_title[error.data.exception_type] !== 'Warning' ? (" - " + map_title[error.data.exception_type]) : '', + title: map_title[error.data.exception_type] !== 'Warning' ? map_title[error.data.exception_type] : '', }) }); } - this.show_warning(error); - //InternalError - } else { this.show_error(error); } @@ -230,12 +233,14 @@ var CrashManager = core.Class.extend({ return; } var message = error.data ? error.data.message : error.message; - return new WarningDialog(this, { - title: _.str.capitalize(error.type) || _t("Odoo Warning"), - subtitle: error.data.title, - }, { + var title = _.str.capitalize(error.type) || _t("Oops Something went wrong !"); + var subtitle = error.data ? error.data.title : error.title; + this.displayNotification({ + title: title, message: message, - }).open(); + subtitle: subtitle, + sticky: true, + }); }, show_error: function (error) { if (!this.active) { diff --git a/addons/web/static/src/js/services/notification_service.js b/addons/web/static/src/js/services/notification_service.js index ea16d9ef66f..295b49d760c 100644 --- a/addons/web/static/src/js/services/notification_service.js +++ b/addons/web/static/src/js/services/notification_service.js @@ -60,6 +60,7 @@ var NotificationService = AbstractService.extend({ * @param {function} [params.Notification] javascript class of a notification * to instantiate by default use 'web.Notification' * @param {string} params.title notification title + * @param {string} params.subtitle notification subtitle * @param {string} params.message notification main message * @param {string} params.type 'notification' or 'warning' * @param {boolean} [params.sticky=false] if true, the notification will stay diff --git a/addons/web/static/src/js/widgets/notification.js b/addons/web/static/src/js/widgets/notification.js index 04d42abd7af..a93cc97f91d 100644 --- a/addons/web/static/src/js/widgets/notification.js +++ b/addons/web/static/src/js/widgets/notification.js @@ -25,6 +25,7 @@ var Notification = Widget.extend({ * @param {Widget} parent * @param {Object} params * @param {string} params.title + * @param {string} params.subtitle * @param {string} [params.message] * @param {string} [params.type='warning'] 'info', 'success', 'warning', 'danger' or '' * @param {boolean} [params.sticky=false] if true, the notification will @@ -41,6 +42,7 @@ var Notification = Widget.extend({ init: function (parent, params) { this._super.apply(this, arguments); this.title = params.title; + this.subtitle = params.subtitle; this.message = params.message; this.buttons = params.buttons || []; this.sticky = !!this.buttons.length || !!params.sticky; diff --git a/addons/web/static/src/xml/notification.xml b/addons/web/static/src/xml/notification.xml index 3ffe9e8979a..592c3ee23b0 100644 --- a/addons/web/static/src/xml/notification.xml +++ b/addons/web/static/src/xml/notification.xml @@ -16,7 +16,9 @@
-
+
+
From e26d23cb971757787238d05ffaad34619c886dde Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:27:01 +0000 Subject: [PATCH 8/9] [FIX] website_forum: remove unused close notif event This code was not used properly and was triggering useless rpc. This code was used in the deprecated badge system and we forgot to remove it. Commit where the code should have been removed: ec212de5c7a7b0f4b74549add62a2988f5b38ee6 Commit where a quick fix was made: 1e1ea00e66686d76a36fa0c19c3e4865e3161031 --- addons/website_forum/controllers/main.py | 6 ------ .../static/src/js/website_forum.js | 17 ----------------- 2 files changed, 23 deletions(-) diff --git a/addons/website_forum/controllers/main.py b/addons/website_forum/controllers/main.py index 86ce6993a90..272e119f94c 100644 --- a/addons/website_forum/controllers/main.py +++ b/addons/website_forum/controllers/main.py @@ -59,12 +59,6 @@ class WebsiteForum(WebsiteProfile): }) return "/forum/%s" % slug(forum_id) - @http.route('/forum/notification_read', type='json', auth="user", methods=['POST'], website=True) - def notification_read(self, **kwargs): - if kwargs.get('notification_id'): - request.env['mail.message'].browse([int(kwargs.get('notification_id'))]).set_message_done() - return True - def sitemap_forum(env, rule, qs): Forum = env['forum.forum'] dom = sitemap_qs2dom(qs, '/forum', Forum._rec_name) diff --git a/addons/website_forum/static/src/js/website_forum.js b/addons/website_forum/static/src/js/website_forum.js index 1557d6fd50f..51cae0cc28f 100644 --- a/addons/website_forum/static/src/js/website_forum.js +++ b/addons/website_forum/static/src/js/website_forum.js @@ -30,7 +30,6 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ 'click .accept_answer:not(.karma_required)': '_onAcceptAnswerClick', 'click .favourite_question': '_onFavoriteQuestionClick', 'click .comment_delete': '_onDeleteCommentClick', - 'click .notification_close': '_onCloseNotificationClick', 'click .js_close_intro': '_onCloseIntroClick', }, @@ -451,22 +450,6 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ $link.parents('.comment').first().remove(); }); }, - /** - * @private - * @param {Event} ev - */ - _onCloseNotificationClick: function (ev) { - if (!session.is_website_user) { - ev.preventDefault(); - var $link = $(ev.currentTarget); - this._rpc({ - route: '/forum/notification_read', - params: { - notification_id: $link.attr('id'), - }, - }); - } - }, /** * @private * @param {Event} ev From 6e55a3a49666484c38345ba85b14441dc3056ec2 Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Tue, 11 Jun 2019 12:27:05 +0000 Subject: [PATCH 9/9] [IMP] website_forum: display forum errors with notif Previously the Forum errors were handled with a popover, now the errors will be consistent with the crash_manager and display a toast notification via the show_warning method. Also the KarmaError were previously not displaying the karma needed to perform the action. Now it will display the amount in the error message. The CrashManager show_warning method will now take an options dictionary as a second argument to override it's default behaviour. Part of https://github.com/odoo/odoo/pull/32132 task-1894820 --- .../static/src/js/services/crash_manager.js | 6 +- addons/website_forum/models/forum.py | 98 +++++++++++-------- .../static/src/js/website_forum.js | 92 +++++++---------- .../static/src/scss/website_forum.scss | 6 -- 4 files changed, 93 insertions(+), 109 deletions(-) diff --git a/addons/web/static/src/js/services/crash_manager.js b/addons/web/static/src/js/services/crash_manager.js index 6a78d776afa..701d97f0a53 100644 --- a/addons/web/static/src/js/services/crash_manager.js +++ b/addons/web/static/src/js/services/crash_manager.js @@ -228,19 +228,19 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, this.show_error(error); } }, - show_warning: function (error) { + show_warning: function (error, options) { if (!this.active) { return; } var message = error.data ? error.data.message : error.message; var title = _.str.capitalize(error.type) || _t("Oops Something went wrong !"); var subtitle = error.data ? error.data.title : error.title; - this.displayNotification({ + this.displayNotification(_.extend({ title: title, message: message, subtitle: subtitle, sticky: true, - }); + }, options)); }, show_error: function (error) { if (!this.active) { diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 23d1608bb01..f75bb69ebe8 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -384,7 +384,7 @@ class Post(models.Model): filter_regexp = r'()|(]*?href[^>]*?>)|(<[a-z|A-Z]+[^>]*style\s*=\s*[\'"][^\'"]*\s*background[^:]*:[^url;]*url)' content_match = re.search(filter_regexp, content, re.I) if content_match: - raise KarmaError('User karma not sufficient to post an image or link.') + raise KarmaError(_('%d karma required to post an image or link.') % forum.karma_editor) return content def _default_website_meta(self): @@ -412,9 +412,9 @@ class Post(models.Model): raise UserError(_('Posting answer on a [Deleted] or [Closed] question is not possible.')) # karma-based access if not post.parent_id and not post.can_ask: - raise KarmaError('You don\'t have enough karma to create a new question.') + raise KarmaError(_('%d karma required to create a new question.') % post.forum_id.karma_ask) elif post.parent_id and not post.can_answer: - raise KarmaError('You don\'t have enough karma to answer a question.') + raise KarmaError(_('%d karma required to answer a question.') % post.forum_id.karma_answer) if not post.parent_id and not post.can_post: post.sudo().state = 'pending' @@ -429,8 +429,9 @@ class Post(models.Model): # XDO FIXME: to be correctly fixed with new get_mail_message_access and filter access rule if operation in ('write', 'unlink') and (not model_name or model_name == 'forum.post'): # Make sure only author or moderator can edit/delete messages - if any(not post.can_edit for post in self.browse(res_ids)): - raise KarmaError('Not enough karma to edit a post.') + for post in self.browse(res_ids): + if not post.can_edit: + raise KarmaError(_('%d karma required to edit a post.') % post.karma_edit) return super(Post, self).get_mail_message_access(res_ids, operation, model_name=model_name) @api.multi @@ -438,33 +439,37 @@ class Post(models.Model): trusted_keys = ['active', 'is_correct', 'tag_ids'] # fields where security is checked manually if 'content' in vals: vals['content'] = self._update_content(vals['content'], self.forum_id.id) - if 'state' in vals: - if vals['state'] in ['active', 'close']: - if any(not post.can_close for post in self): - raise KarmaError('Not enough karma to close or reopen a post.') - trusted_keys += ['state', 'closed_uid', 'closed_date', 'closed_reason_id'] - elif vals['state'] == 'flagged': - if any(not post.can_flag for post in self): - raise KarmaError('Not enough karma to flag a post.') - trusted_keys += ['state', 'flag_user_id'] - if 'active' in vals: - if any(not post.can_unlink for post in self): - raise KarmaError('Not enough karma to delete or reactivate a post') - if 'is_correct' in vals: - if any(not post.can_accept for post in self): - raise KarmaError('Not enough karma to accept or refuse an answer') - # update karma except for self-acceptance - mult = 1 if vals['is_correct'] else -1 - for post in self: + + tag_ids = False + if 'tag_ids' in vals: + tag_ids = set(tag.get('id') for tag in self.resolve_2many_commands('tag_ids', vals['tag_ids'])) + + for post in self: + if 'state' in vals: + if vals['state'] in ['active', 'close']: + if not post.can_close: + raise KarmaError(_('%d karma required to close or reopen a post.') % post.karma_close) + trusted_keys += ['state', 'closed_uid', 'closed_date', 'closed_reason_id'] + elif vals['state'] == 'flagged': + if not post.can_flag: + raise KarmaError(_('%d karma required to flag a post.') % post.forum_id.karma_flag) + trusted_keys += ['state', 'flag_user_id'] + if 'active' in vals: + if not post.can_unlink: + raise KarmaError(_('%d karma required to delete or reactivate a post.') % post.karma_unlink) + if 'is_correct' in vals: + if not post.can_accept: + raise KarmaError(_('%d karma required to accept or refuse an answer.') % post.karma_accept) + # update karma except for self-acceptance + mult = 1 if vals['is_correct'] else -1 if vals['is_correct'] != post.is_correct and post.create_uid.id != self._uid: post.create_uid.sudo().add_karma(post.forum_id.karma_gen_answer_accepted * mult) self.env.user.sudo().add_karma(post.forum_id.karma_gen_answer_accept * mult) - if 'tag_ids' in vals: - tag_ids = set(tag.get('id') for tag in self.resolve_2many_commands('tag_ids', vals['tag_ids'])) - if any(set(post.tag_ids.ids) != tag_ids for post in self) and any(self.env.user.karma < post.forum_id.karma_edit_retag for post in self): - raise KarmaError(_('Not enough karma to retag.')) - if any(key not in trusted_keys for key in vals) and any(not post.can_edit for post in self): - raise KarmaError('Not enough karma to edit a post.') + if tag_ids: + if set(post.tag_ids.ids) != tag_ids and self.env.user.karma < post.forum_id.karma_edit_retag: + raise KarmaError(_('%d karma required to retag.') % post.forum_id.karma_edit_retag) + if any(key not in trusted_keys for key in vals) and not post.can_edit: + raise KarmaError(_('%d karma required to edit a post.') % post.karma_edit) res = super(Post, self).write(vals) @@ -566,7 +571,7 @@ class Post(models.Model): @api.one def validate(self): if not self.can_moderate: - raise KarmaError('Not enough karma to validate a post') + raise KarmaError(_('%d karma required to validate a post.') % self.forum_id.karma_moderate) # if state == pending, no karma previously added for the new question if self.state == 'pending': @@ -583,7 +588,7 @@ class Post(models.Model): @api.one def refuse(self): if not self.can_moderate: - raise KarmaError(_('Not enough karma to refuse a post')) + raise KarmaError(_('%d karma required to refuse a post.') % self.forum_id.karma_moderate) self.moderator_id = self.env.user return True @@ -591,7 +596,7 @@ class Post(models.Model): @api.one def flag(self): if not self.can_flag: - raise KarmaError('Not enough karma to flag a post') + raise KarmaError(_('%d karma required to flag a post.') % self.forum_id.karma_flag) if(self.state == 'flagged'): return {'error': 'post_already_flagged'} @@ -607,7 +612,7 @@ class Post(models.Model): @api.one def mark_as_offensive(self, reason_id): if not self.can_moderate: - raise KarmaError('Not enough karma to mark a post as offensive') + raise KarmaError(_('%d karma required to mark a post as offensive.') % self.forum_id.karma_moderate) # remove some karma _logger.info('Downvoting user <%s> for posting spam/offensive contents', self.create_uid) @@ -638,8 +643,9 @@ class Post(models.Model): @api.multi def unlink(self): - if any(not post.can_unlink for post in self): - raise KarmaError('Not enough karma to unlink a post') + for post in self: + if not post.can_unlink: + raise KarmaError(_('%d karma required to unlink a post.') % post.karma_unlink) # if unlinking an answer with accepted answer: remove provided karma for post in self: if post.is_correct: @@ -687,7 +693,7 @@ class Post(models.Model): # karma-based action check: use the post field that computed own/all value if not self.can_comment_convert: - raise KarmaError('Not enough karma to convert an answer to a comment') + raise KarmaError(_('%d karma required to convert an answer to a comment.') % self.karma_comment_convert) # post the message question = self.parent_id @@ -719,10 +725,16 @@ class Post(models.Model): return False # karma-based action check: must check the message's author to know if own / all - karma_convert = comment.author_id.id == self.env.user.partner_id.id and post.forum_id.karma_comment_convert_own or post.forum_id.karma_comment_convert_all + is_author = comment.author_id.id == self.env.user.partner_id.id + karma_own = post.forum_id.karma_comment_convert_own + karma_all = post.forum_id.karma_comment_convert_all + karma_convert = is_author and karma_own or karma_all can_convert = self.env.user.karma >= karma_convert if not can_convert: - raise KarmaError('Not enough karma to convert a comment to an answer') + if is_author and karma_own < karma_all: + raise KarmaError(_('%d karma required to convert your comment to an answer.') % karma_own) + else: + raise KarmaError(_('%d karma required to convert a comment to an answer.') % karma_all) # check the message's author has not already an answer question = post.parent_id if post.parent_id else post @@ -754,7 +766,7 @@ class Post(models.Model): karma_unlink = comment.author_id.id == user.partner_id.id and self.forum_id.karma_comment_unlink_own or self.forum_id.karma_comment_unlink_all can_unlink = user.karma >= karma_unlink if not can_unlink: - raise KarmaError('Not enough karma to unlink a comment') + raise KarmaError(_('%d karma required to unlink a comment.') % karma_unlink) return comment.unlink() @api.multi @@ -803,7 +815,7 @@ class Post(models.Model): self.ensure_one() if not self.can_comment: - raise KarmaError(_('Not enough karma to comment')) + raise KarmaError(_('%d karma required to comment.') % self.karma_comment) if not kwargs.get('record_name') and self.parent_id: kwargs['record_name'] = self.parent_id.name return super(Post, self).message_post(message_type=message_type, **kwargs) @@ -903,9 +915,9 @@ class Vote(models.Model): def _check_karma_rights(self, upvote=None): # karma check if upvote and not self.post_id.can_upvote: - raise KarmaError('You don\'t have enough karma to upvote.') + raise KarmaError(_('%d karma required to upvote.') % self.post_id.forum_id.karma_upvote) elif not upvote and not self.post_id.can_downvote: - raise KarmaError('You don\'t have enough karma to downvote.') + raise KarmaError(_('%d karma required to downvote.') % self.post_id.forum_id.karma_downvote) def _vote_update_karma(self, old_vote, new_vote): if self.post_id.parent_id: @@ -941,5 +953,5 @@ class Tags(models.Model): def create(self, vals): forum = self.env['forum.forum'].browse(vals.get('forum_id')) if self.env.user.karma < forum.karma_tag_create: - raise KarmaError(_('You don\'t have enough karma to create a new Tag.')) + raise KarmaError(_('%d karma required to create a new Tag.') % forum.karma_tag_create) return super(Tags, self.with_context(mail_create_nolog=True, mail_create_nosubscribe=True)).create(vals) diff --git a/addons/website_forum/static/src/js/website_forum.js b/addons/website_forum/static/src/js/website_forum.js index 51cae0cc28f..e25be65ba48 100644 --- a/addons/website_forum/static/src/js/website_forum.js +++ b/addons/website_forum/static/src/js/website_forum.js @@ -2,12 +2,12 @@ odoo.define('website_forum.website_forum', function (require) { 'use strict'; var core = require('web.core'); +var crash_manager = require('web.crash_manager'); var Wysiwyg = require('web_editor.wysiwyg.root'); var publicWidget = require('web.public.widget'); var session = require('web.session'); var utils = require('web.utils'); var qweb = core.qweb; -var WebsiteProfile = require('website_profile.website_profile'); var _t = core._t; @@ -185,20 +185,20 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ } ev.preventDefault(); var msg = karma + ' ' + _t("karma is required to perform this action. "); + var title = _t("Karma Error"); if (forum_id) { msg += '' + _t("Read the guidelines to know how to gain karma.") + ''; } if (session.is_website_user) { msg = _t("Sorry you must be logged in to perform this action"); + title = _t("Access Denied"); } - var $warning = $('
' + - '' + - msg + '
'); - var $voteAlert = $('#karma_alert'); - if ($voteAlert.length) { - $voteAlert.remove(); - } - $karma.after($warning); + crash_manager.show_warning({ + message: msg, + title: title, + }, { + sticky: false, + }); }, /** * @private @@ -281,35 +281,20 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ route: $link.data('href') || ($link.attr('href') !== '#' && $link.attr('href')) || $link.closest('form').attr('action'), }).then(function (data) { if (data.error) { - var $warning; + var message; if (data.error === 'anonymous_user') { - $warning = $( - '
' + - '' + - _t("Sorry you must be logged to flag a post") + - '
' - ); + message = _t("Sorry you must be logged to flag a post"); } else if (data.error === 'post_already_flagged') { - $warning = $( - '
' + - '' + - _t("This post is already flagged") + - '' + - '
' - ); + message = _t("This post is already flagged"); } else if (data.error === 'post_non_flaggable') { - $warning = $( - '
' + - '' + - _t("This post can not be flagged") + - '' + - '
' - ); - } - var $flagAlert = $link.parent().find('#flag_alert'); - if ($flagAlert.length === 0) { - $link.parent().append($warning); + message = _t("This post can not be flagged"); } + crash_manager.show_warning({ + message: message, + title: _t("Access Denied"), + }, { + sticky: false, + }); } else if (data.success) { var elem = $link; if (data.success === 'post_flagged_moderator') { @@ -337,22 +322,18 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ route: $link.data('href'), }).then(function (data) { if (data.error) { - var $warning; + var message; if (data.error === 'own_post') { - $warning = $('
' + - '' + - _t('Sorry, you cannot vote for your own posts') + - '
'); + message = _t('Sorry, you cannot vote for your own posts'); } else if (data.error === 'anonymous_user') { - $warning = $('
' + - '' + - _t('Sorry you must be logged to vote') + - '
'); - } - var $voteAlert = $link.parent().find('#vote_alert'); - if ($voteAlert.length === 0) { - $link.parent().append($warning); + message = _t('Sorry you must be logged to vote'); } + crash_manager.show_warning({ + message: message, + title: _t("Access Denied"), + }, { + sticky: false, + }); } else { $link.parent().find('.vote_count').html(data.vote_count); if (data.user_vote === 0) { @@ -398,17 +379,14 @@ publicWidget.registry.websiteForum = publicWidget.Widget.extend({ }).then(function (data) { if (data.error) { if (data.error === 'anonymous_user') { - var $warning = $( - '
' + - '' + - _t("Sorry, anonymous users cannot choose correct answer.") + - '
' - ); - } - var $correctAnswerAlert = $link.parent().find('#correct_answer_alert'); - if ($correctAnswerAlert.length === 0) { - $link.parent().append($warning); + var message = _t("Sorry, anonymous users cannot choose correct answer."); } + crash_manager.show_warning({ + message: message, + title: _t("Access Denied"), + }, { + sticky: false, + }); } else { $link.toggleClass('oe_answer_true', !!data) .toggleClass('oe_answer_false', !data); diff --git a/addons/website_forum/static/src/scss/website_forum.scss b/addons/website_forum/static/src/scss/website_forum.scss index 018bf679e8a..b6186a77d6e 100644 --- a/addons/website_forum/static/src/scss/website_forum.scss +++ b/addons/website_forum/static/src/scss/website_forum.scss @@ -135,12 +135,6 @@ $bronze: #eea91e; } } - .oe_forum_alert { - position: absolute; - width: 300px; - z-index: 9999; - } - .oe_forum_email_required { position: absolute; margin-top: 155px;