jowilf / starlette-admin

Fast, beautiful and extensible administrative interface framework for Starlette & FastApi applications
https://jowilf.github.io/starlette-admin/
MIT License
606 stars 59 forks source link

Bug: Field can't be visible in one view and not visible in another - attribute is stored in the field itself #581

Open sglebs opened 4 weeks ago

sglebs commented 4 weeks ago

Describe the bug

I used class hierarchies for Views and in a subclass I said that I wanted the same excluded fields from a superclass and some more. Much to my surprise, this subclass code affected the behaviour of the superclass. Upon debugging I found the culprit:

def extract_fields(
    fields: Sequence["BaseField"], action: RequestAction = RequestAction.LIST
) -> Sequence["BaseField"]:
    """Extract fields based on the requested action and exclude flags."""
    arr = []
    for field in fields:
        if (
            (action == RequestAction.LIST and field.exclude_from_list)
            or (action == RequestAction.DETAIL and field.exclude_from_detail)
            or (action == RequestAction.CREATE and field.exclude_from_create)
            or (action == RequestAction.EDIT and field.exclude_from_edit)
        ):
            continue
        arr.append(field)
    return arr

The property of being visible or not is, internally, stored in the field itself - despite the fact that for us View programmers this is expressed as a collection (exclude_fields_from_create, exclude_fields_from_edit, exclude_fields_from_list, exclude_from_detail)

So, programmers are led to believe that viability is based on this collections/lists of fields defined in the class BUT internally the code has the side-effect of tagging the field itself. No other view can even change this visibility.

I believe the correct behaviour would be to test if the field is present in the collection of fields to be excluded. Not cause side-effects in the field itself.

To Reproduce

Environment (please complete the following information):

Additional context I got into this issue when creating multiple views of a Table base don value of a particular column, for polymorphism. More specifically, https://docs.sqlalchemy.org/en/20/orm/inheritance.html#single-table-inheritance . I kept my generic view which sees all kinds of records, but also added custom view for each "record subtype". These custom views would inherit from the generic parent view and add their own restrictions - exclude some fields from visibility. This is when I saw this weird behaviour surface.

sglebs commented 4 weeks ago

My Workaround:

  1. Create a superclass view with my workarounds: class WorakaroundModelView(ModelView)
  2. Define get_fields_list so it calls a polymorphic method in self, instead of a helper method:

    def get_fields_list(
        self,
        request: Request,
        action: RequestAction = RequestAction.LIST,
    ) -> Sequence["BaseField"]:
        """Return a list of field instances to display in the specified view action.
        This function excludes fields with corresponding exclude flags, which are
        determined by the `exclude_fields_from_*` attributes.
    
        Parameters:
             request: The request being processed.
             action: The type of action being performed on the view.
        """
        return self.extract_fields(action)
    1. Inspired in the existing helper function extract_fields , this polymorphic method takes into account the collection of fields in self:
      def extract_fields(self, action: RequestAction = RequestAction.LIST
      ) -> Sequence["BaseField"]:
      """Extract fields based on the requested action and exclude flags."""
      arr = []
      for field in self.fields:
          if (
                  (action == RequestAction.LIST and field.name in self.exclude_fields_from_list)
                  or (action == RequestAction.DETAIL and field.name in self.exclude_fields_from_detail)
                  or (action == RequestAction.CREATE and field.name in self.exclude_fields_from_create)
                  or (action == RequestAction.EDIT and field.name in self.exclude_fields_from_edit)
          ):
              continue
          arr.append(field)
      return arr

Oddly enough I had to use field.name because at this point self.exclude_fields_from_detail etc have strings, not fields.