[PATCH v2 0/4] Fix: Report Overwritten when Provided Name Existing
When user provides an existing debug report name, then the server will generate a new debug report and overwrite the old one. Fixed it in this patch to detect the existing name and ask the user to choose another name. v1 -> v2: 2a) Recovered Generate Button behavior (Wen Wang's comment) 2b) Leveraged get_list() function to check name existence (Aline's comment) Hongliang Wang (4): Add ResourceAlreadyExists Exception (HTTP 409) Add Name Existence Check for Debug Report when Create Add License Statement in kimchi.report_add_main.js Implement UI Part to Properly Handle Report Name Existence src/kimchi/control/base.py | 4 +- src/kimchi/exception.py | 4 ++ src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 5 +- ui/js/src/kimchi.report_add_main.js | 97 +++++++++++++++++++++++-------------- 5 files changed, 73 insertions(+), 38 deletions(-) -- 1.8.1.4
When user tries to create a resource with an existing name, we should return HTTP 409 status code to tell the user and let him/her choose another one. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 2 ++ src/kimchi/exception.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f8a5210..a110ac2 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -276,6 +276,8 @@ class Collection(object): raise cherrypy.HTTPError(400, e.message) except NotFoundError, e: raise cherrypy.HTTPError(404, e.message) + except ResourceAlreadyExists, e: + raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except KimchiException, e: diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index fcf60cc..38daeab 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -91,3 +91,7 @@ class IsoFormatError(KimchiException): class TimeoutExpired(KimchiException): pass + + +class ResourceAlreadyExists(KimchiException): + pass -- 1.8.1.4
Prevent user to overwrite an existing debug report with same name. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 6 +++--- src/kimchi/i18n.py | 1 + src/kimchi/model/debugreports.py | 5 ++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index a110ac2..bca6186 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -27,7 +27,7 @@ from kimchi.control.utils import parse_request, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import KimchiException -from kimchi.exception import MissingParameter, NotFoundError, OperationFailed +from kimchi.exception import MissingParameter, NotFoundError, OperationFailed, ResourceAlreadyExists class Resource(object): @@ -84,6 +84,8 @@ class Resource(object): raise cherrypy.HTTPError(400, e.message) except InvalidOperation, e: raise cherrypy.HTTPError(400, e.message) + except ResourceAlreadyExists, e: + raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except NotFoundError, e: @@ -276,8 +278,6 @@ class Collection(object): raise cherrypy.HTTPError(400, e.message) except NotFoundError, e: raise cherrypy.HTTPError(404, e.message) - except ResourceAlreadyExists, e: - raise cherrypy.HTTPError(409, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) except KimchiException, e: diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 452ede2..90ecc17 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -207,6 +207,7 @@ messages = { "KCHDR0005E": _("Unable to generate debug report %(name)s. Details: %(err)s"), "KCHDR0006E": _("You should give a name for the debug file report."), "KCHDR0007E": _("Name should be a string. Only letters, digits and hyphen ('-') are allowed."), + "KCHDR0008E": _("The debug report with specified name \"%(name)s\" already exists. Please use another one."), "KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"), diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index cd31b31..7994fec 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -26,7 +26,7 @@ import subprocess import time from kimchi import config -from kimchi.exception import KimchiException, NotFoundError, OperationFailed +from kimchi.exception import KimchiException, NotFoundError, OperationFailed, ResourceAlreadyExists from kimchi.model.tasks import TaskModel from kimchi.utils import add_task, kimchi_log from kimchi.utils import run_command @@ -42,6 +42,9 @@ class DebugReportsModel(object): # Generate a name with time and millisec precision, if necessary if ident is None or ident == "": ident = 'report-' + str(int(time.time() * 1000)) + else: + if ident in self.get_list(): + raise ResourceAlreadyExists("KCHDR0008E", {"name": ident}) taskid = self._gen_debugreport_file(ident) return self.task.lookup(taskid) -- 1.8.1.4
Added license statement. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- ui/js/src/kimchi.report_add_main.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js index ad2bb39..28c3e66 100644 --- a/ui/js/src/kimchi.report_add_main.js +++ b/ui/js/src/kimchi.report_add_main.js @@ -1,3 +1,20 @@ +/* + * Project Kimchi + * + * Copyright IBM, Corp. 2014 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ kimchi.report_add_main = function() { var reportGridID = 'available-reports-grid'; var addReportForm = $('#form-report-add'); -- 1.8.1.4
If user provides an existing report name, then tell the error and show generate report window to allow the user to enter a new name. Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- ui/js/src/kimchi.report_add_main.js | 80 ++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js index 28c3e66..269e9d5 100644 --- a/ui/js/src/kimchi.report_add_main.js +++ b/ui/js/src/kimchi.report_add_main.js @@ -17,9 +17,30 @@ */ kimchi.report_add_main = function() { var reportGridID = 'available-reports-grid'; + var generateButton = $('#' + reportGridID + '-generate-button'); var addReportForm = $('#form-report-add'); var submitButton = $('#button-report-add'); var nameTextbox = $('input[name="name"]', addReportForm); + if(kimchi._reportName) { + nameTextbox.val(kimchi._reportName); + kimchi._reportName = null; + delete kimchi._reportName; + } + nameTextbox.select(); + + /* + * FIXME: + * Currently, all buttons will be disabled when a report is being + * generated. Though operations on existing debug reports shouldn't + * be affected when a new one is being generated, and it's expected + * to enable Rename/Remove/Download Buttons whenever users click an + * existing report row in the grid. + */ + var disableToolbarButtons = function(event, toEnable) { + $('#' + reportGridID + ' .grid-toolbar button') + .prop('disabled', !toEnable); + }; + var submitForm = function(event) { if(submitButton.prop('disabled')) { return false; @@ -32,43 +53,25 @@ kimchi.report_add_main = function() { } var formData = addReportForm.serializeObject(); kimchi.window.close(); - $('#' + reportGridID + '-generate-button').prop('disabled',true); - $('#' + reportGridID + '-remove-button').prop('disabled',true); - $('#' + reportGridID + '-download-button').prop('disabled',true); - $('#' + reportGridID + '-rename-button').prop('disabled',true); - $('.grid-body table tr', '#' + reportGridID).click(function() { - $('#' + reportGridID + '-remove-button').prop('disabled',true); - $('#' + reportGridID + '-download-button').prop('disabled',true); - $('#' + reportGridID + '-rename-button').prop('disabled',true); - }); - var textboxValue = $('#report-name-textbox').val(); - if (textboxValue != "") { - $('.grid-body-view table', '#' + reportGridID).prepend( - '<tr>' + - '<td>' + - '<div class="cell-text-wrapper">' + textboxValue + '</div>' + - '</td>' + - '<td id ="id-debug-img">' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + - '</td>' + - '</tr>' - ); - } - else { - $('.grid-body-view table', '#' + reportGridID).prepend( - '<tr>' + - '<td>' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6012M'] + '</div>' + - '</td>' + - '<td id ="id-debug-img">' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + - '</td>' + - '</tr>' - ); - } + disableToolbarButtons(); + $('.grid-body table tr', '#' + reportGridID) + .on('click', disableToolbarButtons); + var reportName = nameTextbox.val() || i18n['KCHDR6012M']; + $('.grid-body-view table tbody', '#' + reportGridID).prepend( + '<tr>' + + '<td>' + + '<div class="cell-text-wrapper">' + reportName + '</div>' + + '</td>' + + '<td id ="id-debug-img">' + + '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + + '</td>' + + '</tr>' + ); kimchi.createReport(formData, function(result) { $('.grid-body-view table tr:first-child', '#' + reportGridID).remove(); - $('#' + reportGridID + '-generate-button').prop('disabled',false); + $('.grid-body table tr', '#' + reportGridID) + .off('click', disableToolbarButtons); + generateButton.prop('disabled', false); kimchi.topic('kimchi/debugReportAdded').publish({ result: result }); @@ -81,8 +84,13 @@ kimchi.report_add_main = function() { else { var errText = result['responseJSON']['reason']; } - result && kimchi.message.error(errText) + result && kimchi.message.error(errText); $('.grid-body-view table tr:first-child', '#' + reportGridID).remove(); + $('.grid-body table tr', '#' + reportGridID) + .off('click', disableToolbarButtons); + generateButton.prop('disabled', false); + kimchi._reportName = reportName; + $('#' + reportGridID + '-generate-button').click(); }); event.preventDefault(); -- 1.8.1.4
On 06/13/2014 02:15 PM, Hongliang Wang wrote:
If user provides an existing report name, then tell the error and show generate report window to allow the user to enter a new name.
Signed-off-by: Hongliang Wang <hlwang@linux.vnet.ibm.com> --- ui/js/src/kimchi.report_add_main.js | 80 ++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 36 deletions(-)
diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js index 28c3e66..269e9d5 100644 --- a/ui/js/src/kimchi.report_add_main.js +++ b/ui/js/src/kimchi.report_add_main.js @@ -17,9 +17,30 @@ */ kimchi.report_add_main = function() { var reportGridID = 'available-reports-grid'; + var generateButton = $('#' + reportGridID + '-generate-button'); var addReportForm = $('#form-report-add'); var submitButton = $('#button-report-add'); var nameTextbox = $('input[name="name"]', addReportForm); + if(kimchi._reportName) { + nameTextbox.val(kimchi._reportName); + kimchi._reportName = null; + delete kimchi._reportName; + } + nameTextbox.select(); + + /* + * FIXME: + * Currently, all buttons will be disabled when a report is being + * generated. Though operations on existing debug reports shouldn't + * be affected when a new one is being generated, and it's expected + * to enable Rename/Remove/Download Buttons whenever users click an + * existing report row in the grid. + */ + var disableToolbarButtons = function(event, toEnable) { + $('#' + reportGridID + ' .grid-toolbar button') + .prop('disabled', !toEnable); + }; + var submitForm = function(event) { if(submitButton.prop('disabled')) { return false; @@ -32,43 +53,25 @@ kimchi.report_add_main = function() { } var formData = addReportForm.serializeObject(); kimchi.window.close(); - $('#' + reportGridID + '-generate-button').prop('disabled',true); - $('#' + reportGridID + '-remove-button').prop('disabled',true); - $('#' + reportGridID + '-download-button').prop('disabled',true); - $('#' + reportGridID + '-rename-button').prop('disabled',true); - $('.grid-body table tr', '#' + reportGridID).click(function() { - $('#' + reportGridID + '-remove-button').prop('disabled',true); - $('#' + reportGridID + '-download-button').prop('disabled',true); - $('#' + reportGridID + '-rename-button').prop('disabled',true); - }); - var textboxValue = $('#report-name-textbox').val(); - if (textboxValue != "") { - $('.grid-body-view table', '#' + reportGridID).prepend( - '<tr>' + - '<td>' + - '<div class="cell-text-wrapper">' + textboxValue + '</div>' + - '</td>' + - '<td id ="id-debug-img">' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + - '</td>' + - '</tr>' - ); - } - else { - $('.grid-body-view table', '#' + reportGridID).prepend( - '<tr>' + - '<td>' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6012M'] + '</div>' + - '</td>' + - '<td id ="id-debug-img">' + - '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + - '</td>' + - '</tr>' - ); - } + disableToolbarButtons(); + $('.grid-body table tr', '#' + reportGridID) + .on('click', disableToolbarButtons); + var reportName = nameTextbox.val() || i18n['KCHDR6012M']; + $('.grid-body-view table tbody', '#' + reportGridID).prepend( + '<tr>' + + '<td>' + + '<div class="cell-text-wrapper">' + reportName + '</div>' + + '</td>' + + '<td id ="id-debug-img">' + + '<div class="cell-text-wrapper">' + i18n['KCHDR6007M'] + '</div>' + + '</td>' + + '</tr>' + ); kimchi.createReport(formData, function(result) { $('.grid-body-view table tr:first-child', '#' + reportGridID).remove(); - $('#' + reportGridID + '-generate-button').prop('disabled',false); + $('.grid-body table tr', '#' + reportGridID) + .off('click', disableToolbarButtons); + generateButton.prop('disabled', false); kimchi.topic('kimchi/debugReportAdded').publish({ result: result }); @@ -81,8 +84,13 @@ kimchi.report_add_main = function() { else { var errText = result['responseJSON']['reason']; } - result && kimchi.message.error(errText) + result && kimchi.message.error(errText); $('.grid-body-view table tr:first-child', '#' + reportGridID).remove(); + $('.grid-body table tr', '#' + reportGridID) + .off('click', disableToolbarButtons); + generateButton.prop('disabled', false); + kimchi._reportName = reportName; + $('#' + reportGridID + '-generate-button').click(); This line can be substituted with "generateButton.click" since there is such a variable defined to keep the consistence of the code });
event.preventDefault();
participants (2)
-
Hongliang Wang -
Wen Wang