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

chore: Update testing, iconography, and formatting #41

Merged
merged 12 commits into from
Nov 20, 2024
Merged

Conversation

fsbraun
Copy link
Member

@fsbraun fsbraun commented Nov 19, 2024

Prepare release 2.0.0a

#38 brings django CMS 4 support. This PR updates the build pipeline and testing.

Refactor the codebase to update form field names for clarity, migrate build configuration to pyproject.toml, and update CI workflows and test configurations to support newer Python and Django CMS versions.

Enhancements:

  • Refactor the form field 'cms_page' to 'cms_pagecontent' for improved clarity and consistency with the data model.

Build:

  • Migrate build configuration from setup.py to pyproject.toml, aligning with modern Python packaging standards.

CI:

  • Update GitHub Actions workflow to test against Python versions 3.9 to 3.12 and Django CMS 4.1, removing older versions.

Tests:

  • Refactor tests to accommodate changes in form field names and update test configurations to align with new Django CMS and Python versions.

Related resources

Checklist

  • I have opened this pull request against master
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on
    Slack to find a “pr review buddy” who is going to review my pull request.

Summary by Sourcery

Update the project to support Django CMS 4 by refactoring form fields, migrating build configuration to pyproject.toml, and updating CI workflows and test configurations for newer Python and Django CMS versions.

Enhancements:

  • Refactor form field 'cms_page' to 'cms_pagecontent' for improved clarity and consistency with the data model.

Build:

  • Migrate build configuration from setup.py to pyproject.toml, aligning with modern Python packaging standards.

CI:

  • Update GitHub Actions workflow to test against Python versions 3.9 to 3.12 and Django CMS 4.1, removing older versions.

Tests:

  • Refactor tests to accommodate changes in form field names and update test configurations to align with new Django CMS and Python versions.

Copy link
Contributor

sourcery-ai bot commented Nov 19, 2024

Reviewer's Guide by Sourcery

This PR modernizes the codebase to support Django CMS 4.x by updating build configurations, testing infrastructure, and form field names. The changes include migrating from setup.py to pyproject.toml, updating CI workflows for newer Python/Django versions, and refactoring form fields for better clarity.

ER diagram for form field changes

erDiagram
    EXPORT_IMPORT_FORM {
        ModelChoiceField cms_pagecontent
    }
    PAGE_CONTENT {
        admin_manager latest_content
    }
    EXPORT_IMPORT_FORM }|--|| PAGE_CONTENT : uses
    note for EXPORT_IMPORT_FORM "Refactored 'cms_page' to 'cms_pagecontent'"
    note for PAGE_CONTENT "Updated queryset to use 'admin_manager.latest_content()'"
Loading

Updated class diagram for ExportImportForm

classDiagram
    class ExportImportForm {
        +ModelChoiceField cms_pagecontent
        +clean()
    }
    class PluginExportForm {
        +get_filename()
        +run_export()
    }
    class PluginImportForm {
        +clean()
        +run_import()
    }
    ExportImportForm <|-- PluginExportForm
    ExportImportForm <|-- PluginImportForm
    note for ExportImportForm "Refactored 'cms_page' to 'cms_pagecontent'"
    note for PluginExportForm "Updated methods to use 'cms_pagecontent'"
    note for PluginImportForm "Updated methods to use 'cms_pagecontent'"
Loading

File-Level Changes

Change Details Files
Migrated build configuration from setup.py to pyproject.toml
  • Moved package metadata and configuration to pyproject.toml
  • Updated Python version requirements to >=3.9
  • Updated Django CMS requirement to >=4.1
  • Removed setup.cfg in favor of pyproject.toml configuration
setup.py
pyproject.toml
setup.cfg
Updated testing infrastructure for newer framework versions
  • Modified tox configuration to test against Python 3.9-3.12
  • Updated test environments for Django 4.2, 5.0, and 5.1
  • Updated GitHub Actions workflow to use newer Python versions
  • Replaced djangocms_text_ckeditor with djangocms_text
  • Added CMS_CONFIRM_VERSION4 setting
tox.ini
.github/workflows/test.yml
tests/settings.py
tests/requirements/*
Refactored form field names and related functionality
  • Renamed 'cms_page' field to 'cms_pagecontent' for clarity
  • Updated form validation and processing logic for the renamed field
  • Modified test cases to use new field names and PageContent model
djangocms_transfer/forms.py
djangocms_transfer/cms_toolbars.py
tests/transfer/test_forms.py
tests/transfer/abstract.py
Updated iconography implementation
  • Replaced PNG icons with inline SVG icons
  • Updated CSS to use data URLs for icons
  • Removed background positioning in favor of direct SVG replacement
djangocms_transfer/static/djangocms_transfer/css/transfer.css

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @fsbraun - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 4 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

djangocms_transfer/importer.py Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
djangocms_transfer/forms.py Show resolved Hide resolved
Comment on lines +76 to 86
if placeholder and (cms_pagecontent or plugin):
message = _(
"Plugins can be imported to pages, plugins or placeholders. Not all three."
)
raise forms.ValidationError(message)

if plugin and (cms_page or placeholder):
message = _("Plugins can be imported to pages, plugins or placeholders. Not all three.")
if plugin and (cms_pagecontent or placeholder):
message = _(
"Plugins can be imported to pages, plugins or placeholders. Not all three."
)
raise forms.ValidationError(message)
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Remove redundant conditional [×2] (remove-redundant-if)

djangocms_transfer/forms.py Show resolved Hide resolved
djangocms_transfer/forms.py Show resolved Hide resolved
fsbraun and others added 2 commits November 19, 2024 22:13
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 63.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 57.92%. Comparing base (83890e2) to head (7a27194).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
djangocms_transfer/forms.py 73.33% 3 Missing and 1 partial ⚠️
djangocms_transfer/cms_plugins.py 0.00% 3 Missing ⚠️
djangocms_transfer/cms_toolbars.py 0.00% 2 Missing ⚠️
djangocms_transfer/compat.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   62.12%   57.92%   -4.21%     
==========================================
  Files          11       11              
  Lines         404      404              
  Branches       61       62       +1     
==========================================
- Hits          251      234      -17     
- Misses        137      152      +15     
- Partials       16       18       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@fsbraun fsbraun requested a review from vinitkumar November 19, 2024 21:26
@fsbraun
Copy link
Member Author

fsbraun commented Nov 19, 2024

@sourcery-ai review

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @fsbraun - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

README.rst Show resolved Hide resolved
djangocms_transfer/forms.py Show resolved Hide resolved
djangocms_transfer/forms.py Show resolved Hide resolved
djangocms_transfer/forms.py Show resolved Hide resolved
@fsbraun fsbraun merged commit a9efa18 into master Nov 20, 2024
28 checks passed
@fsbraun fsbraun deleted the chore/testing branch November 20, 2024 13:17
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