4:16 a.m.
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.
v2 -> v3:
3a) Reduced code redundant
(Wen Wang's comment)
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
4:16 a.m.
New subject: [PATCH v3 1/4] Add ResourceAlreadyExists Exception (HTTP 409)
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(a)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
6:18 a.m.
New subject: [PATCH v3 1/4] Add ResourceAlreadyExists Exception (HTTP 409)
On 06/13/2014 06:16 AM, Hongliang Wang wrote:
And let's keep using InvalidParameter for this proposal and if we get
agreement 409 is better we change it for all the resources.
That way we keep consistence between them.
4:16 a.m.
New subject: [PATCH v3 2/4] Add Name Existence Check for Debug Report when Create
Prevent user to overwrite an existing debug report with same name.
Signed-off-by: Hongliang Wang <hlwang(a)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
4:16 a.m.
New subject: [PATCH v3 3/4] Add License Statement in kimchi.report_add_main.js
Added license statement.
Signed-off-by: Hongliang Wang <hlwang(a)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
4:16 a.m.
New subject: [PATCH v3 4/4] Implement UI Part to Properly Handle Report Name Existence
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(a)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..112b37d 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;
+ generateButton.click();
});
event.preventDefault();
--
1.8.1.4
8:21 a.m.
New subject: [PATCH v3 4/4] Implement UI Part to Properly Handle Report Name Existence
On 06/13/2014 08:18 PM, Aline Manera wrote:
I think Wen Wang will fix it. It was introduced in the patch for
generating loading icon:
* [PATCH V5 0/2] Issue#296 UI redesign when generating debug reports
11:08 a.m.
On 06/14/2014 10:24 AM, Hongliang Wang wrote:
No.
I mean, to the error be displayed the screen blinks.
1) Select generate a new debug report
2) A pop-up will be shown
3) Enter an existent name for the debug report
4) The screen will blink (like a reload page) then the error message is
shown
3999
days inactive
4002
days old
12 comments
2 participants
participants (2)
-
Aline Manera -
Hongliang Wang