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

fix: Delay importing models.CMSPlugin in utils. #637

Merged
merged 3 commits into from
Nov 30, 2022

Conversation

mbi
Copy link
Contributor

@mbi mbi commented Nov 29, 2022

Continuation of PR #636

As mentioned in #468, Importing html.clean_html too early prevents registering custom user models containing an HTMLField.

This solves the issue by decoupling the import of models.CMSPlugin into the relevant function calls, to allow registering the models properly.

NOTE: This lacks a proper test, as I couldn't manage to register a custom user model and define it in settings.AUTH_USER_MODEL using django-cms's app_helper mechanism. If you absolutely need a test, can you please provide guidance on how to define a custom user model, that would trigger the error (when 172e1f0348415fa80ce4b622c34587460e162afd is not present)?

…ctions, this allows adding an HTMLField into a custom user model
@mbi mbi marked this pull request as draft November 29, 2022 20:38
@mbi mbi marked this pull request as ready for review November 29, 2022 20:42
@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Merging #637 (4a86f6a) into master (9ac4c15) will increase coverage by 0.06%.
The diff coverage is 72.22%.

❗ Current head 4a86f6a differs from pull request most recent head 21459f6. Consider uploading reports for the commit 21459f6 to get more accurate results

@@            Coverage Diff             @@
##           master     #637      +/-   ##
==========================================
+ Coverage   69.86%   69.93%   +0.06%     
==========================================
  Files          16       16              
  Lines         448      449       +1     
  Branches       49       49              
==========================================
+ Hits          313      314       +1     
  Misses        118      118              
  Partials       17       17              
Impacted Files Coverage Δ
djangocms_text_ckeditor/utils.py 89.24% <72.22%> (+0.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@fsbraun fsbraun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@fsbraun
Copy link
Member

fsbraun commented Nov 30, 2022

Some of the test fail due to migrations of other packages. This is fixed in #631. So do not worry about that.

Can you fix the isort issue (see linting results)? I'll merge then.

Thanks very much!!

@fsbraun fsbraun merged commit c8d5fce into django-cms:master Nov 30, 2022
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.

2 participants