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

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 this issue Sep 24, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@sglebs
Copy link

sglebs commented Sep 24, 2024

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

  • Create a View which defines for example exclude_fields_from_edit.
  • Define a subclass of this view, which adds some more fields to be excluded
  • Show the original superclass view at runtime and you will see that the fields excluded from the subclass view will also be excluded from this parent View too.

Environment (please complete the following information):

  • Starlette-Admin version: 0.14.0
  • ORM/ODMs: SQLAlchemy, psycopg2-binary==2.9.9
  • SQLAlchemy-serializer==1.4.1

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 sglebs added the bug Something isn't working label Sep 24, 2024
@sglebs sglebs changed the title Bug: Field can't be visible in one view and not visible in another - attributed is stored in the field itself Bug: Field can't be visible in one view and not visible in another - attribute is stored in the field itself Sep 24, 2024
@sglebs
Copy link
Author

sglebs commented Sep 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant