Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMSPluginBase should use force_text and force_str for __str__ and __repr__ #5034

Closed
zanderle opened this issue Feb 26, 2016 · 5 comments
Closed

Comments

@zanderle
Copy link
Contributor

When working on aldryn_bootstrap3 plugin, I stumbled upon a following error:

In [18]: c = CMSPlugin.objects.get(id=6061)
In [24]: c.get_plugin_class()
Out[24]: aldryn_bootstrap3.cms_plugins.Bootstrap3ColumnCMSPlugin

In [21]: c.get_plugin_name()
Out[21]: <django.utils.functional.__proxy__ at 0x7fd8d4e2a610>

In [22]: c.get_plugin_instance()
Out[22]: ---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/core/formatters.pyc in __call__(self, obj)
    697                 type_pprinters=self.type_printers,
    698                 deferred_pprinters=self.deferred_printers)
--> 699             printer.pretty(obj)
    700             printer.flush()
    701             return stream.getvalue()

/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/lib/pretty.pyc in pretty(self, obj)
    366                 if cls in self.type_pprinters:
    367                     # printer registered in self.type_pprinters
--> 368                     return self.type_pprinters[cls](obj, self, cycle)
    369                 else:
    370                     # deferred printer

/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/lib/pretty.pyc in inner(obj, p, cycle)
    550                 p.text(',')
    551                 p.breakable()
--> 552             p.pretty(x)
    553         if len(obj) == 1 and type(obj) is tuple:
    554             # Special case for 1-item tuples.

/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/lib/pretty.pyc in pretty(self, obj)
    381                             if callable(meth):
    382                                 return meth(obj, self, cycle)
--> 383             return _default_pprint(obj, self, cycle)
    384         finally:
    385             self.end_group()

/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/lib/pretty.pyc in _default_pprint(obj, p, cycle)
    501     if _safe_getattr(klass, '__repr__', None) not in _baseclass_reprs:
    502         # A user-provided repr. Find newlines and replace them with p.break_()
--> 503         _repr_pprint(obj, p, cycle)
    504         return
    505     p.begin_group(1, '<')

/home/zan/.virtualenvs/datafy-cms/local/lib/python2.7/site-packages/IPython/lib/pretty.pyc in _repr_pprint(obj, p, cycle)
    692     """A pprint that just redirects to the normal repr function."""
    693     # Find newlines and replace them with p.break_()
--> 694     output = repr(obj)
    695     for idx,output_line in enumerate(output.splitlines()):
    696         if idx:

TypeError: __repr__ returned non-string (type __proxy__)

This exception also gets raised when trying to publish some changes. Looking at the code, the problem seems to be in CMSPluginBase : https://github.com/divio/django-cms/blob/develop/cms%2Fplugin_base.py#L347-L351

Because __str__ and __repr__ don't use force_text and force_str, the lazy translation causes an error. Therefore, CMSPluginBase should probably use force_text and force_str for __str__ and __repr__

@zanderle
Copy link
Contributor Author

Or maybe get_plugin_name on CMSPlugin shouldn't access name attribute on CMSPluginBase directly: https://github.com/divio/django-cms/blob/develop/cms/models/pluginmodel.py#L119

So instead of

def get_plugin_name(self):
    from cms.plugin_pool import plugin_pool

    return plugin_pool.get_plugin(self.plugin_type).name

it should be something like

def get_plugin_name(self):
    from cms.plugin_pool import plugin_pool

    return smart_text(plugin_pool.get_plugin(self.plugin_type))

Assuming that it's ok to have name translatable...

@stefanfoulis
Copy link
Contributor

@yakky @mkoistinen Do you guys see a problem in using force_text and force_str in the baseclass. As far as I can tell there is no sense in keeping the strings lazy here anyway, since this will be called at runtime and should pick up the current language correctly.

Something we can squeeze in 3.2.2? or wait for 3.3?

@czpython
Copy link
Contributor

czpython commented May 17, 2016

I think both the __repr__ and __str__ should return always the same representation regardless of language, so I think we shouldn't use the name attribute and instead use the plugin type which would be self.__class__.__name__. Only when displaying plugins to end users should we use name.

@divio/django-cms-core thoughts?

@yakky
Copy link
Member

yakky commented May 17, 2016

@czpython @stefanfoulis IIRC __str__ is used to render the plugin name in some part of the user interface. __repr__ is definitely internal, though

@Aiky30
Copy link
Contributor

Aiky30 commented Dec 8, 2020

Appears to have been resolved and merged in the PR referenced above.

@Aiky30 Aiky30 closed this as completed Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants