5monkeys/django-bananas

Django extensions the monkey way

django-bananas's Issues

LoginView missing CSRF protection

Correct me if I'm wrong or missing something, although:

It seems to me that the LoginView.create is lacking CSRF protection.

Citing the DRF docs regarding SessionAuthentication:

CSRF validation in REST framework works slightly differently to standard Django due to the need to support both session and non-session based authentication to the same views. This means that only authenticated requests require CSRF tokens, and anonymous requests may be sent without CSRF tokens. This behaviour is not suitable for login views, which should always have CSRF validation applied.

We can see that DRF's GenericViewSet inherits skipping of CSRF checking and the SessionAuthentication class skips CSRF checking as the login view's permission class requires the user to be anonymous. (LoginView inherits the SessionAuthentication from here)

The csrf_protect decorator should probably be set similarly to how Django's LoginView does it.

Feature proposal: SafeEnvironWrapper

When writing applications that take configuration from the environment it's often a good idea to err on failing fast if the application isn't correctly configured. The current implementation of EnvironWrapper lies on the other side of the spectrum, choosing to return None if values aren't found. This moves the responsibility of validating configuration unnecessarily deep into applications.

I propose creating a new class, SafeEnvironWrapper that will throw an exception when values aren't found and no default is provided. This allows the parsing methods to be accurately typed as returning only an instance of the specific type when no default is provided.

This also requires having a static type for UNDEFINED. We can achieve that either with NewType or with a class:

Undefined = NewType("Undefined", object)
UNDEFINED = Undefined(object())

class _Undefined: ...
UNDEFINED = _Undefined()

Expose translations in the bananas API

Admin API: Registering nested routes?

In certain situations, it'd be helpful to be able to register additional (nested) routers to the main BananasRouter admin router. OpenAPI supports having path parameters, which is possibly how the generated schema should end up being defined using for those nested endpoints, if swagger js client supports it (for django-bananas.js).

Scrolling bug in popups

https://www.dropbox.com/s/tv2m6do7j2j00ul/bug.mov?dl=0

ExtendedQuerySet doesn't return all possible combinations when renaming reverse relation fields

This only affects the Django1.9+ version of ExtendedQuerySet.

Using the Parent/Child models used in the tests as an example:

>>> p = Parent.objects.create(name="P")
>>> c1 = Child.objects.create(parent=p, name="C1")
>>> c2 = Child.objects.create(parent=p, name="C2")

>>> Parent.objects.filter(name="P").dicts(child_name="child__name")
[{'child_name': 'C1'}, {}]

>>> # It only affects renamed fields
>>> Parent.objects.filter(name="P").dicts("child__name")
[{'child__name': 'C1'}, {'child__name': 'C2'}]

Attempted fix #10

The search bar looks bad in a popup

Have drf-yasg as an extra only

I'm getting drf-yasg with all of its dependencies included while not using more than bananas.environment, bananas.models and bananas.admin.extension. Due to drf-yasg in install_requires:

django-bananas/setup.cfg

Lines 35 to 38 in 8e832ca

	install_requires =
	Django>=2.2
	typing-extensions>=3.7.4.3
	drf-yasg>=1.20.0

Could perhaps drf-yasg move to only be installed with extras_require.drf?

Feature proposal: "augmented list"

I've recently stumbled into a simliar pattern twice where I want to configure a list such that one can pass in partial objects that only contain a primary key, but have fleshed out objects in the response. In the example below we're creating (or PUT-ing) an object that has relations to already existing entities, and we don't want to allow the client to alter the related entities in this request, but we want the client to conveniently be able to get the full objects in the response.

Example request

{
    "related_entities": [
        {"id": 1},
        {"id": 123},
    ]
}

Example response

{
    "related_entities": [
        {"id": 1, "name": "foo"},
        {"id": 123, "name": "bar"},
    ]
}

Solution

For lack of a better name, I'm calling this an "augmented list" for now. Suggestions are welcome.

class AugmentedList(ListField):
    def __init__(self, child: Serializer, method_name: str | None = None, writable_field: str = "id") -> None:
        super().__init__(source="*", child=child, default=list)
        self.method_name = method_name
        self.writable_field = writable_field

    def to_internal_value(  # type: ignore[override]
        self, data: Iterable[dict[str, int]]
    ) -> dict[str, Iterable[int]]:
        assert self.field_name is not None
        return {self.field_name: [item[self.writable_field] for item in data]}
    
    # Stolen from SerializerMethodField
    def bind(self, field_name, parent):
        # The method name defaults to `get_{field_name}`.
        if self.method_name is None:
            self.method_name = 'get_{field_name}'.format(field_name=field_name)
        super().bind(field_name, parent)

    # Stolen from SerializerMethodField and modified to use self.child for serialization and to not pass
    # any value to the method.
    def to_representation(self, value):
        method = getattr(self.parent, self.method_name)
        return [
            type(self.child)(instance=child_instance).data
            for child_instance in method()
        ]

The above specialized list field allows us to achieve the behavior we want, and would be used like this:

class TagSerializer(Serializer):
    id = IntegerField()
    name = CharField()

class HasTags(Serializer):
    tags = AugmentedList(child=TagSerializer())

    def get_tags(self):
        return Tag.objects.filter(...=self.instance.pk)

(The above code is untested as-is, but all the concepts are fully working in separate places at the moment).

syncpermissions should update name

Changing verbose_name of a AdminView causes duplicate key error when running syncpermissions

django.db.utils.IntegrityError: duplicate key value violates unique constraint "auth_permission_content_type_id_codename_01ab375a_uniq"
DETAIL:  Key (content_type_id, codename)=(58, can_access_x) already exists.

p, created = Permission.objects.get_or_create(
    codename=codename, name=name, content_type=ct
)

should probably be changed to

p, created = Permission.objects.update_or_create(
    codename=codename, content_type=ct, defaults=dict(name=name)
)

Can that cause any unintented issues?