[PATCH saem_ref] Avoid 404 on /<etype> when there's no entity yet (CONSEJIRA-511)

Frank Bessou frank.bessou at logilab.fr
Wed Sep 19 10:50:57 CEST 2018



On 19/09/2018 10:16, Denis Laxalde wrote:
> Frank Bessou a écrit :
>>
>>
>> On 18/09/2018 14:44, Denis Laxalde wrote:
>>> Frank Bessou a écrit :
>>>> I have questions about parts of this changeset.
>>>>
>>>> On 18/09/2018 12:05, Denis Laxalde wrote:
>>>>> # HG changeset patch
>>>>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>>>>> # Date 1536852213 -7200
>>>>> #      Thu Sep 13 17:23:33 2018 +0200
>>>>> # Node ID 0f20032cc3fee0837be58b580005cc334e9b508e
>>>>> # Parent  6a8cc01c6ba71a7612af26ab5202d7c8ceea6219
>>>>> # Available At http://hg.logilab.org/review/cubes/saem_ref
>>>>> #              hg pull http://hg.logilab.org/review/cubes/saem_ref -r
>>>>> 0f20032cc3fe
>>>>> # EXP-Topic CONSEJIRA-511
>>>>> Avoid 404 on /<etype> when there's no entity yet (CONSEJIRA-511)
>>>>>
>>>>> The default behavior of CubicWeb is to raise a NotFound exception when
>>>>> the result set is empty on a /<etype> route. This is not very user
>>>>> friendly (not even sure this is semantically correct). So we override
>>>>> this behavior and display an informative message with an invite for
>>>>> entity creation (if possible accounting for user rights). The same
>>>>> should be handled on custom routes (e.g. /sedalib).
>>>>>
>>>>> This is achieved by overriding RestPathEvaluator.set_vid_for_rset()
>>>>> (where the NotFound exception is raised) to set the view identifier to
>>>>> "noresult" when the result set is empty. Class SAEMNoResultView
>>>>> (implementing "noresult" regid) replaces the default one and renders
>>>>> the
>>>>> message.
>>>>>
>>>
>>>>>     diff --git a/cubicweb_saem_ref/views/__init__.py
>>>>> b/cubicweb_saem_ref/views/__init__.py
>>>>> --- a/cubicweb_saem_ref/views/__init__.py
>>>>> +++ b/cubicweb_saem_ref/views/__init__.py
>>>>> @@ -26,10 +26,23 @@ from cubicweb import NoSelectableObject,
>>>>>     from cubicweb.utils import json_dumps, js_href
>>>>>     from cubicweb.uilib import domid, js
>>>>>     from cubicweb.view import EntityView
>>>>> -from cubicweb.predicates import (has_permission, is_instance,
>>>>> multi_lines_rset, match_kwargs,
>>>>> -                                 partial_has_related_entities)
>>>>> +from cubicweb.predicates import (
>>>>> +    has_permission,
>>>>> +    is_instance,
>>>>> +    match_kwargs,
>>>>> +    multi_lines_rset,
>>>>> +    partial_has_related_entities,
>>>>> +)
>>>>>     from cubicweb.web import component, formwidgets as fw
>>>>> -from cubicweb.web.views import basecomponents, uicfg, urlrewrite,
>>>>> tabs, primary
>>>>> +from cubicweb.web.views import (
>>>>> +    basecomponents,
>>>>> +    baseviews,
>>>>> +    primary,
>>>>> +    tabs,
>>>>> +    uicfg,
>>>>> +    urlrewrite,
>>>>> +    urlpublishing,
>>>>> +)
>>>>>       from cubes.squareui.views.basetemplates import basetemplates
>>>>>     from cubicweb_seda.views import dropdown_button, has_rel_perm,
>>>>> widgets as sedawidgets
>>>>> @@ -429,11 +442,54 @@ class URLAttributeView(primary.URLAttrib
>>>>>                 self.w(u'<a href="%s">%s</a>' % (url, url))
>>>>>       +class SAEMNoResultView(baseviews.NoResultView):
>>>>> +    """Enhanced "noresult" view with a nicer message and an invite to
>>>>> create
>>>>> +    an entity when permission is granted.
>>>>> +    """
>>>>> +
>>>>> +    def call(self, **kwargs):
>>>>> +        vreg = self._cw.vreg
>>>>> +        try:
>>>>> +            etype = self.cw_rset.description[0][0] > +        except
>>>>> IndexError:
>>>>> +            # rset probably comes from a user RQL query, fall back to
>>>>> default
>>>>> +            # behavior.
>>>>> +            return super(SAEMNoResultView, self).call(**kwargs)
>>>>
>>>> I don't understand what represents cw_rset.description for an empty
>>>> result set. Why description[0][0] when the query is made by a user ?
>>>
>>> If the rset is empty and corresponds to a single entity type request
>>> (e.g. "Any X WHERE X is Thing, ..."), the description will contain that
>>> entity as first item. Otherwise (the rset is still empty, per selection
>>> context), we cannot retrieve the entity from rset description (because
>>> it's empty, hence "except IndexError:") so we fall back to the default
>>> behavior. I mentioned "user RQL query" because this view will be
>>> selected for e.g. /?rql=Any X WHERE X name "toto"; but this is probably
>>> not the only case it would.
>>>
>>>>
>>>>> +        assert etype in vreg.schema, etype
>>>>> +        msg = self._cw._('No entity of type "{etype}" yet.').format(
>>>>> +            etype=self._cw.__(etype)
>>>>> +        )
>>>>> +        self.w(u'<div class="searchMessage">{}'.format(
>>>>> +            xml_escape(msg)))
>>>>> +        if vreg.schema[etype].has_perm(self._cw, 'add'):
>>>>> +            self.w(u' <a href="{url}">{invite}</a>'.format(
>>>>> +
>>>>> url=vreg['etypes'].etype_class(etype).cw_create_url(self._cw),
>>>>> +                invite=xml_escape(self._cw._('Create one?')),
>>>>> +            ))
>>>>> +        self.w(u'</div>')
>>>>
>>>> Why not using the with statement here ?
>>>>
>>>> with div(w, _class="searchMessage") as d:
>>>>       d(xml_escape(msg))
>>>>       ...
>>>
>>> There's no such context manager in cubicweb. It only exists in cwtags,
>>> which is not used in this project.
>>
>> Ok :)
>>
>>>
>>>>
>>>>> +
>>>>> +
>>>>> +class RestPathEvaluator(urlpublishing.RestPathEvaluator):
>>>>> +
>>>>> +    def set_vid_for_rset(self, req, cls, rset):
>>>>> +        if rset.rowcount == 0:
>>>>> +            req.form['vid'] = 'noresult'
>>>>> +            return
>>>>> +        return super(RestPathEvaluator, self).set_vid_for_rset(req,
>>>>> cls, rset)
>>>>
>>>> After having read:
>>>> https://docs.cubicweb.org/book/devweb/views/urlpublish.html#cubicweb.web.views.urlpublishing.RestPathEvaluator
>>>>
>>>>
>>>> I don't understand why we enter this evaluator.
>>>> Is sedalib an entity type ?
>>>
>>> No, sedalib is a route that runs an RQL query, hence we'll get to the
>>> 'noresult' view directly from there. (There's a link in main site
>>> navigation to this route.)
>>>
>>
>> Ok, thanks for the explanation. :)
>>
>>
>>
>>> Change of RestPathEvaluator concerns requests to /<etype> only.
>>>
>>
>> Why does SAEMNoResultView doesn't get selected by default on /etype ?
> 
> Because the RestPathEvaluator raises a NotFound exception if the result
> set is empty, so we get the "notfound" view, not the "noresult" view.
> This is what I explain in the first paragraph of the commit message.
> 
> The confusion comes from me mixing two closely related changes in the
> same changeset:
> 
> - I want requests on /<etype> producing an empty rset to not return a
> 404 but a custom view suggesting to create the first entity; this is
> solved by tweaking the RestPathEvaluator (to not raise NotFound) *and*
> setting the "vid" to be ultimately selected. Here this could have been a
> custom view, not the "noresult" view; in fact, this is what I initially
> implemented until I realized that...
> 
> - I want requests on custom routes, like /sedalib, to behave similarly.
> However, the code path is not the same in these cases and the "noresult"
> view is directly selected. Hence I decided to select "noresult" from
> RestPathEvaluator and this solves two issues with a single view.
> 
> 
> Perhaps I should split the changeset and explain things better.
> 
> 

Thanks for the explanation, I understand now :p.
I would be happy to apply this changeset with the explanation you just
gave as commit message (splitted or not). I think it is clearer than the
current message :)

>> I would have expected it to be used since you replaced NoResultView by
>> SAEMNoResultView at registration.
>>
>>>>
>>>>> +
>>>>> +
>>>>>     def registration_callback(vreg):
>>>>>         from cubicweb.web.views import actions, cwuser, tableview,
>>>>> undohistory
>>>>> -    vreg.register_all(globals().values(), __name__,
>>>>> (URLAttributeView, SAEMHTMLPageFooter,))
>>>>> +    vreg.register_all(globals().values(), __name__, (
>>>>> +        RestPathEvaluator,
>>>>> +        SAEMHTMLPageFooter,
>>>>> +        SAEMNoResultView,
>>>>> +        URLAttributeView,
>>>>> +    ))
>>>>> +    vreg.register_and_replace(RestPathEvaluator,
>>>>> urlpublishing.RestPathEvaluator)
>>>>>         vreg.register_and_replace(URLAttributeView,
>>>>> primary.URLAttributeView)
>>>>>         vreg.register_and_replace(SAEMHTMLPageFooter,
>>>>> basetemplates.HTMLPageFooter)
>>>>> +    vreg.register_and_replace(SAEMNoResultView,
>>>>> baseviews.NoResultView)
>>>>>         vreg.unregister(tableview.TableView)
>>>>>         vreg.unregister(undohistory.UndoHistoryView)
>>>>>         vreg.unregister(basecomponents.ApplicationName)
>>>
>>
> 

-- 
Frank Bessou
Logilab         https://www.logilab.fr



More information about the saem-devel mailing list