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

[Resolves #468] Move clean_html import into clean methods #469

Closed
wants to merge 2 commits into from

Conversation

danielsamuels
Copy link

This change allows the HTMLField to be used within a custom user model without causing a LookupError.

This change allows the HTMLField to be used within a custom user model without causing a LookupError.
@coveralls
Copy link

coveralls commented Mar 7, 2018

Coverage Status

Coverage increased (+0.03%) to 78.043% when pulling d980bb9 on danielsamuels:patch-1 into bba7d19 on divio:master.

@mbi
Copy link
Contributor

mbi commented Nov 28, 2022

Hello @danielsamuels 👋 I've been bitten a few times by #468 and I've stumbled upon this PR of yours. I'd like to reopen that issue and get it fixed. May I ask why you closed this PR? Was the fix not effective?

Thanks.

@fsbraun fsbraun reopened this Nov 28, 2022
@fsbraun fsbraun closed this Nov 28, 2022
@fsbraun
Copy link
Sponsor Member

fsbraun commented Nov 28, 2022

Hi @mbi ! Since the htmllib's sanitizer has been deprecated it is time to move to bleach as a sanitizer for djangocms-text-ckeditor. I'd suggest to refactor the imports together with this change. What do you think? Would you be able to support such a PR?

@mbi
Copy link
Contributor

mbi commented Nov 28, 2022

@fsbraun yes, I'll submit a PR as soon as I find a moment...

@fsbraun
Copy link
Sponsor Member

fsbraun commented Nov 29, 2022

Since the discussion in #632 might take some time to resolve, does it make sense for now, to move

from cms.models import CMSPlugin

from the top of the file into the get_plugins_from_text function - just as the other cms core import in that function?

def get_plugins_from_text(text, regex=OBJ_ADMIN_RE):
from cms.utils.plugins import downcast_plugins
plugin_ids = plugin_tags_to_id_list(text, regex)
plugins = CMSPlugin.objects.filter(pk__in=plugin_ids).select_related('placeholder')
plugin_list = downcast_plugins(plugins, select_placeholder=True)
return {plugin.pk: plugin for plugin in plugin_list}

I'd assume this would solve the issue? Also, it would be great to have a test to check this...

@fsbraun fsbraun reopened this Nov 29, 2022
@fsbraun fsbraun closed this Nov 29, 2022
@mbi
Copy link
Contributor

mbi commented Nov 29, 2022

@fsbraun it absolutely solves the issue, I'm running a fork just to do that. I'll prepare a PR just for this issue, and I'll analyze how hard it'd be to actually test this.

@mbi
Copy link
Contributor

mbi commented Nov 29, 2022

@fsbraun here you go: #636. As mentioned in the PR, I couldn't manage to test this. If a test is strictly required, I'll need help on how to setup a custom user model within django-cms' app_help / testing mechanism. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants