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

add integration with django-autocomplete-light #74

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MRoci
Copy link
Contributor

@MRoci MRoci commented Nov 13, 2020

Since the improvements due to the autocompletion feature are fundamental to have an usable user experience with a very large user base i think it would be nice to have django-autocomplete-light support already built into the library as well as for django-ajax-selects.
This could lead in a smoother integration and having fewer dependencies to mantain in projects that heavily rely on dal.

This PR looks for a setting key called SU_DAL_VIEW_NAME and if present it tries to use the dal autocompletion widget in the form.

Maybe the configuration work documented in the README could be reduced if we brought the autocompletion view into the library as well

@coveralls
Copy link

coveralls commented Nov 13, 2020

Coverage Status

Coverage: 91.85% (-2.6%) from 94.426% when pulling 8b7394c on MRoci:dal-integration into aadabf6 on adamcharnock:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 5cd146c on MRoci:dal-integration into 856efc8 on adamcharnock:develop.

Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

If you rebase tests should hopefully pass

README.rst Outdated Show resolved Hide resolved
@PetrDlouhy
Copy link
Collaborator

PetrDlouhy commented Apr 1, 2022

I have updated the whole codebase and added testing in all supported versions of Django. Could you please rebase this to the current develop branch and fix the tests, if there are any problems?

@MRoci
Copy link
Contributor Author

MRoci commented Apr 21, 2023

@PetrDlouhy with a little bit of latency i've rebased this PR against current develop 😅

@PetrDlouhy
Copy link
Collaborator

@MRoci I merged #83, now you can rebase once more to see if the tests are passing.

@MRoci MRoci force-pushed the dal-integration branch 2 times, most recently from eade02f to 8b7394c Compare April 21, 2023 15:59
@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2023

Codecov Report

Patch coverage: 43.75% and project coverage change: -2.57 ⚠️

Comparison is base (aadabf6) 94.46% compared to head (eade02f) 91.90%.

❗ Current head eade02f differs from pull request most recent head 8b7394c. Consider uploading reports for the commit 8b7394c to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #74      +/-   ##
===========================================
- Coverage    94.46%   91.90%   -2.57%     
===========================================
  Files            9        9              
  Lines          307      321      +14     
===========================================
+ Hits           290      295       +5     
- Misses          17       26       +9     
Impacted Files Coverage Δ
django_su/forms.py 55.26% <20.00%> (-12.60%) ⬇️
django_su/views.py 89.65% <83.33%> (-1.09%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@PetrDlouhy
Copy link
Collaborator

@MRoci Thank you very much for your contribution.

Can you please also add some tests for the new code?

adds UserSuDalForm form class which leverages the autocomplete
facilities provided by django-autocomplete-light.
It expects:
  * 'django-light-autocomplete' to be installed
  * 'dal' to be present into INSTALLED_APPS
  * 'SU_DAL_VIEW_NAME' setting to contain the name of the autocomplete view

If 'SU_DAL_VIEW_NAME' setting key is present and form_class is default su_login view tries to
automatically use UserSuDalForm, avoiding the end user to customize
django_su's urls.

Required configuration is added into README
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.

5 participants