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

feat: added re-usable Link widget and JSON endpoint #229

Merged
merged 94 commits into from
Nov 20, 2024
Merged

Conversation

fsbraun
Copy link
Member

@fsbraun fsbraun commented Oct 25, 2024

Description

This PR implements the result of discussion django-cms/django-cms#8024 and prepares for django CMS Link 5.0.

  • Re-usable LinkWidget, LinkFormField, and LinkField (model field)
  • Re-usable JSON endpoint for available URLs
  • Improved usability (a single link widget instead of two fieldsets consisting of 6 fields to enter a link)
  • The site dropdown (autocomplete field) only appears in multi-site installations
  • Automatic detection of linkable models (Django or django CMS) which have admins with search_fieldsattribute and which have a get_absolute_url method
  • Migrations (tested in unit tests!) automatically update existing plugins
  • Fixes get_link will always return absolute urls instead of relative urls, making it hard to author content/test out locally #200

preview

Other changes include the update of translations and tests to current Django 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.

Copy link

codecov bot commented Oct 26, 2024

Codecov Report

Attention: Patch coverage is 94.12879% with 31 lines in your changes missing coverage. Please review.

Project coverage is 95.35%. Comparing base (1d33608) to head (221201b).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
djangocms_link/admin.py 86.95% 12 Missing and 6 partials ⚠️
djangocms_link/fields.py 96.72% 1 Missing and 5 partials ⚠️
djangocms_link/migrations/0017_link_link.py 85.18% 1 Missing and 3 partials ⚠️
djangocms_link/helpers.py 97.43% 0 Missing and 2 partials ⚠️
djangocms_link/apps.py 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
+ Coverage   93.81%   95.35%   +1.54%     
==========================================
  Files          24       27       +3     
  Lines         291      668     +377     
  Branches       27       86      +59     
==========================================
+ Hits          273      637     +364     
- Misses         11       14       +3     
- Partials        7       17      +10     

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


🚨 Try these New Features:

@fsbraun fsbraun changed the title feat: Link widget and JSON endpoints feat: Link widget and JSON endpoint Nov 10, 2024
@marksweb marksweb self-assigned this Nov 10, 2024
@marksweb marksweb self-requested a review November 10, 2024 13:02
@fsbraun fsbraun changed the title feat: Link widget and JSON endpoint feat: added re-usable Link widget and JSON endpoint Nov 10, 2024
tests/requirements/base.txt Show resolved Hide resolved
pyproject.toml Outdated
[build-system]
build-backend = "hatchling.build"

requires = [ "hatchling" ]
Copy link
Member

Choose a reason for hiding this comment

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

I think this sets this project apart from others. Is this something you want to start using across the board?

Copy link
Member Author

Choose a reason for hiding this comment

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

Frankly, that's just convenience: hatchling created the pyproject.toml file automatically. I can see if I can configure it to use setuptools instead.

Copy link
Member

Choose a reason for hiding this comment

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

I haven't used hatchling, so I'm not against it and don't really have any strong opinions on packaging options. So I was mostly curious and I know Jens Erik talked about hatchling a while back.

Copy link
Member Author

Choose a reason for hiding this comment

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

From what I've seen, the discovery is somewhat smarter. You don't need a MANIFEST.in file. I guess setuptools grew quite complex over the years. I changed things to setuptools, let's see how it works.

tox.ini Outdated
{env:COMMAND:coverage} report

[testenv:flake8]
deps = flake8
deps =
Copy link
Member

Choose a reason for hiding this comment

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

We could consider dropping everything but tests from tox. Because pre-commit and github actions can pick up these.

Then you can get the github tests to install with uv and install tox and tox-uv. This is a similar setup then to what I have here; https://github.com/marksweb/django-nh3/blob/main/.github/workflows/main.yml#L42

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really use tox. Kept it for consistency. I'm fine with removing linting from tox. uv probably is for performance reasons, right?

Copy link
Member

Choose a reason for hiding this comment

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

Tox just makes running the test suite locally easier - then you can get github to do the same for consistency.

Yeah - uv is just doing things better. (I actually used it to setup a fresh project to show an example of clearing cache from admin recently.)

@fsbraun
Copy link
Member Author

fsbraun commented Nov 20, 2024

@sourcery-ai review

Copy link

@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.

We failed to fetch the diff for pull request #229

You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.

@fsbraun fsbraun merged commit 6d7af0d into master Nov 20, 2024
41 checks passed
@fsbraun fsbraun deleted the feat/url-mngr branch November 20, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants