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

Frank Bessou frank.bessou at logilab.fr
Tue Sep 18 14:14:45 CEST 2018


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/i18n/en.po b/cubicweb_saem_ref/i18n/en.po
> --- a/cubicweb_saem_ref/i18n/en.po
> +++ b/cubicweb_saem_ref/i18n/en.po
> @@ -75,6 +75,9 @@ msgstr ""
>   msgid "Citation_plural"
>   msgstr "Citations"
>   
> +msgid "Create one?"
> +msgstr ""
> +
>   msgid "Edit authority settings"
>   msgstr ""
>   
> @@ -121,6 +124,10 @@ msgstr ""
>   msgid "New OrganizationUnit"
>   msgstr ""
>   
> +#, python-brace-format
> +msgid "No entity of type \"{etype}\" yet."
> +msgstr ""
> +
>   msgid "Organization"
>   msgstr ""
>   
> diff --git a/cubicweb_saem_ref/i18n/fr.po b/cubicweb_saem_ref/i18n/fr.po
> --- a/cubicweb_saem_ref/i18n/fr.po
> +++ b/cubicweb_saem_ref/i18n/fr.po
> @@ -81,6 +81,9 @@ msgstr ""
>   msgid "Citation_plural"
>   msgstr "Citations"
>   
> +msgid "Create one?"
> +msgstr "En créer une ?"
> +
>   msgid "Edit authority settings"
>   msgstr "Configurer l'autorité administrative"
>   
> @@ -127,6 +130,10 @@ msgstr "Nouvelle autorité administrative"
>   msgid "New OrganizationUnit"
>   msgstr "Nouvelle unité administrative"
>   
> +#, python-brace-format
> +msgid "No entity of type \"{etype}\" yet."
> +msgstr "Aucune entité de type \"{etype}\" pour l'instant."
> +
>   msgid "Organization"
>   msgstr "Autorité administrative"
>   
> 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 ?

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

> +
> +
> +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 ?

> +
> +
>   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)
> diff --git a/test/test_views.py b/test/test_views.py
> --- a/test/test_views.py
> +++ b/test/test_views.py
> @@ -625,5 +625,48 @@ class TimelineViewsTC(CubicWebTC):
>                                      ar1.view('incontext', w=None)])
>   
>   
> +class BaseViewsTC(CubicWebTC):
> +    """Test for non-business views."""
> +    configcls = PostgresApptestConfiguration
> +
> +    def expected_notfound_html(self, etype, addlink=False):
> +        parts = [
> +            '<div class="searchMessage">No entity of type "{}" yet.'.format(etype)
> +        ]
> +        if addlink:
> +            parts.append(
> +                ' <a href="http://testing.fr/cubicweb/add/{}">Create one?</a>'.format(etype)
> +            )
> +        parts.append('</div>')
> +        return ''.join(parts)
> +
> +    def test_no_notfound_when_no_entity(self):
> +        """Check that we do not get 404 at /<etype> when no entity of <etype>
> +        exists yet.
> +        """
> +        with self.admin_access.web_request() as req:
> +            # Request /bookmark, usually there's none on a fresh instance.
> +            result = self.app_handle_request(req, path='/bookmark')
> +            self.assertIn(self.expected_notfound_html('Bookmark', True), result) > +        with self.new_access('anon').web_request() as req:
> +            result = self.app_handle_request(req, path='/bookmark')
> +            self.assertIn(self.expected_notfound_html('Bookmark'), result)
> +
> +    def test_no_notfound_sedalib(self):
> +        with self.admin_access.web_request() as req:
> +            # Request /bookmark, usually there's none on a fresh instance.
> +            result = self.app_handle_request(req, path='/sedalib')
> +            self.assertIn(self.expected_notfound_html('SEDAArchiveUnit', True), result)
> +        with self.new_access('anon').web_request() as req:
> +            result = self.app_handle_request(req, path='/sedalib')
> +            self.assertIn(self.expected_notfound_html('SEDAArchiveUnit'), result)
> +
> +    def test_true_404(self):
> +        """Check 404 is returned on unknown entity type."""
> +        with self.admin_access.web_request() as req:
> +            self.app_handle_request(req, path='/zorglub')
> +            self.assertEqual(req.status_out, 404)
> +
> +
>   if __name__ == '__main__':
>       unittest.main()
> 

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



More information about the saem-devel mailing list