[IMP] significantly improve handling of trying to create a new page which already exists

# Through the menu

Rather than output a generic error (500 due to an IntegrityError),
return a templated 409 letting the user either visit the
page-which-already-exists-with-the-name-he'd-picked or open the page
creation dialog again for a second round.

An alternative would be to promote the website.prompt use to
full-blown dialog as a result of selecting "New Page" and handle the
"page already exists" case without even closing the dialog.

# Through the link dialog

Added an RPC call to check if a page already exists (essentially a
cheaper version of opening /page/whatever and checking whether it's a
200 over JSON-RPC), if the page matching the completion term already
exists, do *not* add the choice of creating the page, only provide
completions from existing pages.

bzr revid: xmo@openerp.com-20131126160148-wrk4zvz9istscwa0
This commit is contained in:
Xavier Morel 2013-11-26 17:01:48 +01:00
parent b3061b9103
commit 44af6d1e09
5 changed files with 103 additions and 27 deletions

View File

@ -53,7 +53,12 @@ class Website(openerp.addons.web.controllers.main.Home):
path = web.new_page(request.cr, request.uid, path, context=request.context) path = web.new_page(request.cr, request.uid, path, context=request.context)
except psycopg2.IntegrityError: except psycopg2.IntegrityError:
logger.exception('Unable to create ir_model_data for page %s', path) logger.exception('Unable to create ir_model_data for page %s', path)
return werkzeug.exceptions.InternalServerError() response = request.website.render('website.creation_failed', {
'page': path,
'path': '/page/' + request.website.page_for_name(name=path)
})
response.status_code = 409
return response
url = "/page/" + path url = "/page/" + path
if noredirect is not NOPE: if noredirect is not NOPE:
return werkzeug.wrappers.Response(url, mimetype='text/plain') return werkzeug.wrappers.Response(url, mimetype='text/plain')

View File

@ -143,6 +143,20 @@ class website(osv.osv):
cr.execute("ROLLBACK TO SAVEPOINT new_page") cr.execute("ROLLBACK TO SAVEPOINT new_page")
raise raise
def page_for_name(self, cr, uid, ids, name, module='website', context=None):
# whatever
return '%s.%s' % (module, slugify(name, max_length=50))
def page_exists(self, cr, uid, ids, name, module='website', context=None):
page = self.page_for_name(cr, uid, ids, name, module=module, context=context)
try:
return self.get_template(
cr, uid, ids, template=page, context=context
).exists()
except:
return False
def get_public_user(self, cr, uid, context=None): def get_public_user(self, cr, uid, context=None):
if not self.public_user: if not self.public_user:
uid = openerp.SUPERUSER_ID uid = openerp.SUPERUSER_ID
@ -185,14 +199,14 @@ class website(osv.osv):
}) })
def get_template(self, cr, uid, ids, template, context=None): def get_template(self, cr, uid, ids, template, context=None):
IMD = self.pool.get("ir.model.data") IMD = self.pool["ir.model.data"]
try: try:
module, xmlid = template.split('.', 1) module, xmlid = template.split('.', 1)
view_ref = IMD.get_object_reference(cr, uid, module, xmlid) model, id = IMD.get_object_reference(cr, uid, module, xmlid)
except ValueError: # catches both unpack errors and gor errors except ValueError: # catches both unpack errors and gor errors
module, xmlid = 'website', template module, xmlid = 'website', template
view_ref = IMD.get_object_reference(cr, uid, module, xmlid) model, id = IMD.get_object_reference(cr, uid, module, xmlid)
return self.pool.get("ir.ui.view").browse(cr, uid, view_ref[1]) return self.pool["ir.ui.view"].browse(cr, uid, id, context=context)
def _render(self, cr, uid, ids, template, values=None, context=None): def _render(self, cr, uid, ids, template, values=None, context=None):
user = self.pool.get("res.users") user = self.pool.get("res.users")

View File

@ -888,7 +888,8 @@
this._super(editor); this._super(editor);
this.text = null; this.text = null;
// Store last-performed request to be able to cancel/abort it. // Store last-performed request to be able to cancel/abort it.
this.req = null; this.page_exists_req = null;
this.search_pages_req = null;
}, },
start: function () { start: function () {
var self = this; var self = this;
@ -896,15 +897,20 @@
minimumInputLength: 1, minimumInputLength: 1,
placeholder: _t("New or existing page"), placeholder: _t("New or existing page"),
query: function (q) { query: function (q) {
self.fetch_pages(q.term).then(function (results) { $.when(
self.page_exists(q.term),
self.fetch_pages(q.term)
).then(function (exists, results) {
var rs = _.map(results, function (r) { var rs = _.map(results, function (r) {
return { id: r.url, text: r.name, }; return { id: r.url, text: r.name, };
}); });
rs.push({ if (!exists) {
create: true, rs.push({
id: q.term, create: true,
text: _.str.sprintf(_t("Create page '%s'"), q.term), id: q.term,
}); text: _.str.sprintf(_t("Create page '%s'"), q.term),
});
}
q.callback({ q.callback({
more: false, more: false,
results: rs results: rs
@ -983,20 +989,30 @@
.siblings().removeClass('active') .siblings().removeClass('active')
.addBack().removeClass('has-error'); .addBack().removeClass('has-error');
}, },
fetch_pages: function (term) { call: function (method, args, kwargs) {
var self = this; var self = this;
if (this.req) { this.req.abort(); } var req = method + '_req';
return this.req = openerp.jsonRpc('/web/dataset/call_kw', 'call', {
if (this[req]) { this[req].abort(); }
return this[req] = openerp.jsonRpc('/web/dataset/call_kw', 'call', {
model: 'website', model: 'website',
method: 'search_pages', method: method,
args: [null, term], args: args,
kwargs: { kwargs: kwargs,
limit: 9,
context: website.get_context()
},
}).always(function () { }).always(function () {
// request completed -> unstore it self[req] = null;
self.req = null; });
},
page_exists: function (term) {
return this.call('page_exists', [null, term], {
context: website.get_context(),
});
},
fetch_pages: function (term) {
return this.call('search_pages', [null, term], {
limit: 9,
context: website.get_context(),
}); });
}, },
}); });

View File

@ -72,16 +72,21 @@
<button title="Close" type="button" class="close" data-dismiss="modal">×</button> <button title="Close" type="button" class="close" data-dismiss="modal">×</button>
<div class="h2"><t t-esc="title"/></div> <div class="h2"><t t-esc="title"/></div>
</div> </div>
<div class="modal-body"> <div class="modal-body" t-if="message or backend_url">
<section> <section>
<t t-esc="message"/> <t t-esc="message"/>
</section> </section>
<section class="mt32"> <section class="mt32" t-if="backend_url">
You have encountered an error. You can edit in the <a t-att-href="backend_url">backend</a> this data. <p>The web site has encountered an error.</p>
<p>
It might be possible to edit the relevant items
or fix the issue in <a t-href="#{backend_url}">
the classic OpenERP interface</a>.
</p>
</section> </section>
</div> </div>
<div class="modal-footer"> <div class="modal-footer">
<button type="button" data-dismiss="modal" href="#" class="close btn btn-default">Close</button> <button type="button" data-dismiss="modal" class="btn btn-primary">Close</button>
</div> </div>
</div> </div>
</div> </div>

View File

@ -629,5 +629,41 @@ Sitemap: <t t-esc="url_root"/>sitemap.xml
</t> </t>
</template> </template>
<template id="creation_failed">
<t t-call="website.layout">
<div id="wrap">
<div class="oe_structure">
<div class="container">
<h1 class="text-center">
The page "<em><t t-esc="page"/></em>"
already exists
</h1>
<h3 class="text-center text-muted">We could not create it.</h3>
<ul>
<li>
You can <a t-href="#{path}">visit the existing page</a>
</li>
<li>Or you can <a href="#" class="create-new-page">
create an other page.</a></li>
</ul>
</div>
</div>
</div>
<script type="application/javascript">
$(document.body).on('click', 'a.create-new-page', function (e) {
e.preventDefault();
openerp.website.prompt({
window_title: "New Page",
input: "Page Title",
}).then(function (val) {
if (val) {
document.location = '/pagenew/' + encodeURI(val);
}
});
});
</script>
</t>
</template>
</data> </data>
</openerp> </openerp>