ambient-innovation / ai-django-core Goto Github PK

Description

As of version 5.6.0 of ai-django-core, I believe there is a little room for improvement on these models:

• It uses default=now on created_at and lastmodified_at fields, instead of the good old auto_now=True and auto_now_add=True, which is cleaner and does not make these fields visible/editable on modelForms by default, which is ideal for audit purposes.
• The created_by and lastmodified_by fields should have the editable flag set to False so they are fully automatic and not visible on forms by default.

The current settings are not wrong, but I believe the ones I'm proposing suit more the typical use case for Django Models.

How I believe the fields should look like

created_at = models.DateTimeField(_("Created at"), auto_now_add=True, db_index=True)
created_by = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_("Created by"), blank=True, null=True,
                                   related_name="%(app_label)s_%(class)s_created", on_delete=models.SET_NULL, editable=False)
lastmodified_at = models.DateTimeField(_("Last modified at"), auto_now=True, db_index=True)
lastmodified_by = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_("Last modified by"), blank=True,
                                        null=True, related_name="%(app_label)s_%(class)s_lastmodified",
                                        on_delete=models.SET_NULL, editable=False)

Following the README. Package ai_django_core is installed, app ai_django_core added in setttings.py as INSTALLED_APPS.

When running python manage.py runserver, failed w/ error (see title).

Here is my requirements.txt:

anyjson>=0.3.3
argcomplete>=0.8.7
astroid>=1.4.5
celery>=3.1.23
celerymon>=1.0.3
certifi>=14.5.14
colorama>=0.3.7
coverage>=4.1
defusedxml>=0.4.1
Django>=2.0.0
django-admin-bootstrapped>=2.3.2
django-annoying>=0.8.0
django-appconf>=0.6
django-autocomplete-light>=3.1.4
django-bootstrap3>=7.0.1
django-celery>=3.1.16
django-compressor>=1.4
django-cors-headers>=1.1.0
django-crispy-forms>=1.4.0
django-debug-toolbar>=1.2.2
django-devserver>=0.8.0
django-extensions>=1.6.1
django-extra-views>=0.6.4
django-filter>=0.9.2
django-localflavor>=1.1
django-model-utils>=2.2
django-tastypie>=0.13.3
django-tastypie-swagger>=0.1.4
djangorestframework>=3.3.3
Jinja2>=2.8
psycopg2>=2.8.4
redis>=2.10.3
requests>=2.5.1
requests-oauthlib>=0.4.2
simplejson>=3.6.5
six>=1.9.0
mysqlclient>=1.4.6
numpy
selenium
yahooquery
django-extensions
docutils
pyyaml
newscatcher
gunicorn
ai-django-core

I'm sending an email as txt not html and when i try to use assert_body_contains, this error is raised:

Traceback (most recent call last):
  File "/home/user/workspace/project/user_auth/tests.py", line 35, in test_get_profile
    mails.assert_body_contains('receiving')
  File "/home/user/workspace/project/venv/lib/python3.8/site-packages/ai_django_core/mail/services/tests.py", line 197, in assert_body_contains
    self.assertIn(search_str, self._get_html_content(), msg=msg)
  File "/home/user/workspace/project/venv/lib/python3.8/site-packages/ai_django_core/mail/services/tests.py", line 108, in _get_html_content
    return self._match_list[0].alternatives[0][0]
IndexError: list index out of range

I think there is missing an option to disable the assertIn check in line 197 in services/tests.py or a check if len(alternatives) > 0.

Ensure we just have found one element and then return HTML part of the email

should be:

Ensure we just have found one element and then return body of the email or Return body of the email

Also where is the check that there is only one element?

Hi,

I have recently started using your package after getting introduced to it from Ronny and it has been really helpful in bringing order and structure to my project. So first thank you very much for publishing this package and for maintaining it.

I am currently finding that the unittests for views with DjangoPermissionRequiredMixin fails when the order of permissions changes, i.e., the permission items are the same but with different order. Is assertCountEqual instead of assertEqual a possible option here ? , to compare the permissions defined between test and the actual view class ?

Thanks!

BR