search

django-cms / djangocms-bootstrap4

django CMS Bootstrap 4 is a plugin bundle for django CMS providing several components from the popular Bootstrap 4 framework.
https://www.django-cms.org/
Other
82 stars 58 forks source link

Quick: Fix Grid Container get_short_description for Nested Tuples Setting #145

Closed wesleyboar closed 1 year ago

wesleyboar commented 2 years ago

To Do

Done - [x] (As necessary) Add test cases. ([direction](https://github.com/django-cms/djangocms-bootstrap4/pull/145#issuecomment-1147687443)) ([done by mark](https://github.com/django-cms/djangocms-bootstrap4/pull/145#issuecomment-1148125883) ([merged?](https://github.com/django-cms/djangocms-bootstrap4/pull/145/commits/58372d7d719356a9911db46e21710dd8c2f3997c)) - [x] The `get_short_description` value is inaccurate (see [screenshot](https://user-images.githubusercontent.com/62723358/172020564-e71e542e-1164-42fe-a450-18838bc99c37.png)). (4ba6861, 99dcc07). - [x] Test changes from review (da1f543, 8d8adcb, 4ba6861).

Description

Fix short description for Bootstrap4 Container if user choses container type from nested tuples in DJANGOCMS_BOOTSTRAP4_GRID_CONTAINERS setting.*

_* An <optgroup> is available in "Container Type" <select> field when DJANGOCMS_BOOTSTRAP4_GRID_CONTAINERS has deeply-nested tuples. But were user to chose such an option, then get_short_descritpion would return empty string._

Related resources

Checklist

Testing

  1. Setup a Django CMS and this branch of djangocms-bootstrap4, e.g.
  2. Create a page in the CMS.
  3. Create Container plugin instances on the page for each CONTAINER TYPEs.
  4. ✓ Verify the class in the markup of each element added.
  5. ⊖ Verify the short description (parenthesized value in Structure mode) is accurate.
choice class description
Default → Container container (Container)
Default → Fluid container container-fluid (Fluid container)
Custom → Your container container-yours (Your container)

Screenshots

Bootstrap Grid Container Nested Tuple

[^1]: Added from django.utils.translation import gettext_lazy as _ to your-app/settings.py. [^2]: Added sample DJANGOCMS_BOOTSTRAP4_GRID_CONTAINERS to your-app/settings.py.

codecov-commenter commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@4b5d248). Click here to learn what that means. The diff coverage is 35.00%.

:exclamation: Current head 58372d7 differs from pull request most recent head b3ed5ed. Consider uploading reports for the commit b3ed5ed to get more accurate results

@@            Coverage Diff            @@
##             master     #145   +/-   ##
=========================================
  Coverage          ?   98.93%           
=========================================
  Files             ?       71           
  Lines             ?     1225           
  Branches          ?      160           
=========================================
  Hits              ?     1212           
  Misses            ?        8           
  Partials          ?        5
Impacted Files Coverage Δ
djangocms_bootstrap4/helpers.py 61.76% <23.52%> (ø)
...gocms_bootstrap4/contrib/bootstrap4_grid/models.py 100.00% <100.00%> (ø)

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

crydotsnake commented 2 years ago

Hello @tacc-wbomar, Thanks a lot for your contribution!

Looks good, everything worked for me! 😄

wesleyboar commented 2 years ago

Thank you for the review, @crydotsnake.

How can I accurately describe the status of this PR to dependent parties?

My Effort to Determine Status I reviewed [a contribution doc that mentions "Status"](https://docs.django-cms.org/en/latest/contributing/management.html#status), but I don't see on this PR any of the labels it describes.
wesleyboar commented 2 years ago

If the PR need be cleaned up anyway before merge, feel free to do it or explain the change for me to do.

(I probably Python and Django'd my way around like a child, so I don't mind direction.)

wesleyboar commented 2 years ago

I do not know how to write test cases in Python, Django, DjangoCMS. I am disinclined to learn, because I cannot even run the existing test cases. I have followed the instructions.

I get the console error django.core.exceptions.ImproperlyConfigured: The SECRET_KEY setting must not be empty., even though I added SECRET_KEY = 'barrier to entry too high' to /tests/settings.py.

test case run failure console log ``` source /Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/bin/activate ➜ djangocms-bootstrap4 git:(quick/grid-container-support-nested-tuple-choices) ✗ source /Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/bin/activate (env) ➜ djangocms-bootstrap4 git:(quick/grid-container-support-nested-tuple-choices) ✗ pip install -r tests/requirements.txt ERROR: Could not open requirements file: [Errno 2] No such file or directory: 'tests/requirements.txt' WARNING: You are using pip version 22.0.4; however, version 22.1.2 is available. You should consider upgrading via the '/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/bin/python -m pip install --upgrade pip' command. (env) ➜ djangocms-bootstrap4 git:(quick/grid-container-support-nested-tuple-choices) ✗ pip install -r tests/requirements/base.txt Requirement already satisfied: coverage in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 1)) (6.2) Requirement already satisfied: django-app-helper in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 2)) (3.0.1) Requirement already satisfied: django-treebeard<4.5 in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 3)) (4.4) Requirement already satisfied: tox in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 4)) (3.24.5) Requirement already satisfied: isort in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 6)) (5.10.1) Requirement already satisfied: flake8 in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 7)) (4.0.1) Requirement already satisfied: pyflakes>=2.1 in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 8)) (2.4.0) Requirement already satisfied: wheel in ./env/lib/python3.9/site-packages (from -r tests/requirements/base.txt (line 9)) (0.37.1) Requirement already satisfied: six in ./env/lib/python3.9/site-packages (from django-app-helper->-r tests/requirements/base.txt (line 2)) (1.16.0) Requirement already satisfied: docopt in ./env/lib/python3.9/site-packages (from django-app-helper->-r tests/requirements/base.txt (line 2)) (0.6.2) Requirement already satisfied: dj-database-url in ./env/lib/python3.9/site-packages (from django-app-helper->-r tests/requirements/base.txt (line 2)) (0.5.0) Requirement already satisfied: Django>=2.2 in ./env/lib/python3.9/site-packages (from django-treebeard<4.5->-r tests/requirements/base.txt (line 3)) (4.0.1) Requirement already satisfied: packaging>=14 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (21.3) Requirement already satisfied: pluggy>=0.12.0 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (1.0.0) Requirement already satisfied: toml>=0.9.4 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (0.10.2) Requirement already satisfied: filelock>=3.0.0 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (3.4.2) Requirement already satisfied: virtualenv!=20.0.0,!=20.0.1,!=20.0.2,!=20.0.3,!=20.0.4,!=20.0.5,!=20.0.6,!=20.0.7,>=16.0.0 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (20.13.0) Requirement already satisfied: py>=1.4.17 in ./env/lib/python3.9/site-packages (from tox->-r tests/requirements/base.txt (line 4)) (1.11.0) Requirement already satisfied: pycodestyle<2.9.0,>=2.8.0 in ./env/lib/python3.9/site-packages (from flake8->-r tests/requirements/base.txt (line 7)) (2.8.0) Requirement already satisfied: mccabe<0.7.0,>=0.6.0 in ./env/lib/python3.9/site-packages (from flake8->-r tests/requirements/base.txt (line 7)) (0.6.1) Requirement already satisfied: asgiref<4,>=3.4.1 in ./env/lib/python3.9/site-packages (from Django>=2.2->django-treebeard<4.5->-r tests/requirements/base.txt (line 3)) (3.4.1) Requirement already satisfied: sqlparse>=0.2.2 in ./env/lib/python3.9/site-packages (from Django>=2.2->django-treebeard<4.5->-r tests/requirements/base.txt (line 3)) (0.4.2) Requirement already satisfied: pyparsing!=3.0.5,>=2.0.2 in ./env/lib/python3.9/site-packages (from packaging>=14->tox->-r tests/requirements/base.txt (line 4)) (3.0.6) Requirement already satisfied: platformdirs<3,>=2 in ./env/lib/python3.9/site-packages (from virtualenv!=20.0.0,!=20.0.1,!=20.0.2,!=20.0.3,!=20.0.4,!=20.0.5,!=20.0.6,!=20.0.7,>=16.0.0->tox->-r tests/requirements/base.txt (line 4)) (2.4.1) Requirement already satisfied: distlib<1,>=0.3.1 in ./env/lib/python3.9/site-packages (from virtualenv!=20.0.0,!=20.0.1,!=20.0.2,!=20.0.3,!=20.0.4,!=20.0.5,!=20.0.6,!=20.0.7,>=16.0.0->tox->-r tests/requirements/base.txt (line 4)) (0.3.4) WARNING: You are using pip version 22.0.4; however, version 22.1.2 is available. You should consider upgrading via the '/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/bin/python -m pip install --upgrade pip' command. (env) ➜ djangocms-bootstrap4 git:(quick/grid-container-support-nested-tuple-choices) ✗ python setup.py test running test WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox. running egg_info writing djangocms_bootstrap4.egg-info/PKG-INFO writing dependency_links to djangocms_bootstrap4.egg-info/dependency_links.txt writing requirements to djangocms_bootstrap4.egg-info/requires.txt writing top-level names to djangocms_bootstrap4.egg-info/top_level.txt reading manifest file 'djangocms_bootstrap4.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' adding license file 'LICENSE' writing manifest file 'djangocms_bootstrap4.egg-info/SOURCES.txt' running build_ext Traceback (most recent call last): File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/setup.py", line 44, in setup( File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/setuptools/__init__.py", line 153, in setup return distutils.core.setup(**attrs) File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/core.py", line 148, in setup dist.run_commands() File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/dist.py", line 966, in run_commands self.run_command(cmd) File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/dist.py", line 985, in run_command cmd_obj.run() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/setuptools/command/test.py", line 232, in run self.run_tests() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/setuptools/command/test.py", line 250, in run_tests test = unittest.main( File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/main.py", line 100, in __init__ self.parseArgs(argv) File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/main.py", line 147, in parseArgs self.createTests() File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/main.py", line 158, in createTests self.test = self.testLoader.loadTestsFromNames(self.testNames, File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/loader.py", line 220, in loadTestsFromNames suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/loader.py", line 220, in suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/unittest/loader.py", line 205, in loadTestsFromName test = obj() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/tests/settings.py", line 50, in run runner.cms('djangocms_bootstrap4') File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/app_helper/runner.py", line 49, in cms return runner(argv) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/app_helper/runner.py", line 109, in runner return main(argv) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/app_helper/main.py", line 374, in main return core(args=args, application=application) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/app_helper/main.py", line 299, in core _make_settings(args, application, settings, STATIC_ROOT, MEDIA_ROOT) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/env/lib/python3.9/site-packages/app_helper/utils.py", line 274, in _make_settings django.setup() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/__init__.py", line 24, in setup apps.populate(settings.INSTALLED_APPS) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/apps/registry.py", line 122, in populate app_config.ready() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/admin/apps.py", line 27, in ready self.module.autodiscover() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/admin/__init__.py", line 24, in autodiscover autodiscover_modules('admin', register_to=site) File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/utils/module_loading.py", line 47, in autodiscover_modules import_module('%s.%s' % (app_config.name, module_to_search)) File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/importlib/__init__.py", line 127, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "", line 1030, in _gcd_import File "", line 1007, in _find_and_load File "", line 986, in _find_and_load_unlocked File "", line 680, in _load_unlocked File "", line 850, in exec_module File "", line 228, in _call_with_frames_removed File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/auth/admin.py", line 6, in from django.contrib.auth.forms import ( File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/auth/forms.py", line 11, in from django.contrib.auth.tokens import default_token_generator File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/auth/tokens.py", line 117, in default_token_generator = PasswordResetTokenGenerator() File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/contrib/auth/tokens.py", line 18, in __init__ self.secret = self.secret or settings.SECRET_KEY File "/Users/wbomar/Developer/Code/_demos/djangocms-bootstrap4/.eggs/Django-3.2.11-py3.9.egg/django/conf/__init__.py", line 90, in __getattr__ raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.") django.core.exceptions.ImproperlyConfigured: The SECRET_KEY setting must not be empty. ```
wesleyboar commented 2 years ago

To the PR description, I have added sections: "Known Issues", (manual) "Testing" steps, a "Screenshot".

marksweb commented 2 years ago

@fsbraun were you going to look over this?

fsbraun commented 2 years ago

@wesleyboar @marksweb Sorry, I had the review done put left it pending. So I saw it but you couldn't. Should be fixed now!

fsbraun commented 2 years ago

I remember having problems with testing, too. Can you try to add SECRET_KEY = "totally secret key nobody should know" to https://github.com/django-cms/djangocms-bootstrap4/blob/master/tests/settings.py ? I am not sure this is sufficient, though.

As a thought starter, here's how I tested first_choice. The right place might by test_helpers.py. For testing get_short_description you might want to look at https://github.com/django-cms/djangocms-bootstrap4/blob/master/tests/bootstrap4_grid/test_models.py

wesleyboar commented 2 years ago

I can now run test cases. The problem was that I had not installed Django nor DjangoCMS. I have opened a PR with a change to the README (#152) that may help others.

marksweb commented 2 years ago

I've fixed the tests in master, but can't currently update this PR with the latest changes.

wesleyboar commented 1 year ago

Yes, thank you @marksweb. Thanks all of you working on Django.

I did not have time to get back to this. How anyone does open source and holds a job is astounding.

I used the "Sync fork" ▸ "Update branch" button in GitHub which looks to have also merged the upstream master into this branch. (I don't do much forking, so I'm hoping that button ran the correct command.)

fsbraun commented 1 year ago

@wesleyboar Do not worry about the failing tests. Once #156 is merged you can sync your fork and we can merge this PR!

wesleyboar commented 1 year ago

Thank you. I was. I stop spelunking now.

fsbraun commented 1 year ago

@wesleyboar Can you try update branch once more?