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

Denis Laxalde denis.laxalde at logilab.fr
Wed Sep 19 10:16:12 CEST 2018


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.


> 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)
>>
> 



More information about the saem-devel mailing list