Skip to content

Removed strongly typed View Settings#974

Open
kalisp wants to merge 15 commits into
developfrom
enhancement/open_view_settings
Open

Removed strongly typed View Settings#974
kalisp wants to merge 15 commits into
developfrom
enhancement/open_view_settings

Conversation

@kalisp

@kalisp kalisp commented Jun 11, 2026

Copy link
Copy Markdown
Member

Description of changes

This PR removes strongly typed View Settings for some tabs as it was too cumbersome for frontend to be using them. Any change in FE required same change in BE.

This PR should simplify development.

It is cumbersome for FE using them.
@kalisp kalisp self-assigned this Jun 11, 2026
@kalisp kalisp linked an issue Jun 11, 2026 that may be closed by this pull request
@martastain

Copy link
Copy Markdown
Member

I agree, but the problem is that strong types (using OPModel) do camelCase -> snake_case conversion by default. I am afraid that by removing the types, already stored views will break

@kalisp

kalisp commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I agree, but the problem is that strong types (using OPModel) do camelCase -> snake_case conversion by default. I am afraid that by removing the types, already stored views will break

So should there be some one-time backend DB conversion script to resave it in camelCase or should FE be handling it?

@Innders Innders left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have broken patching of views

[PATCH] http://localhost:3000/api/views/versions/3956a018cf9d4f64b44723bf673bc2d5?project_name=wing_it

{
    "settings": {
        "filter": {
            "operator": "and",
            "conditions": []
        },
        "columns": [],
        "sort_by": null,
        "group_by": null,
        "show_grid": false,
        "sort_desc": false,
        "row_height": 34,
        "grid_height": null,
        "slicer_type": "hierarchy",
        "show_products": false,
        "show_empty_groups": false,
        "group_sort_by_desc": false,
        "featured_version_order": null,
        "showEmptyGroups": false,
        "rowHeight": 34
    }
}
{
    "code": 400,
    "detail": "Request validation error in [PATCH] /api/views/versions/3956a018cf9d4f64b44723bf673bc2d5",
    "path": "/api/views/versions/3956a018cf9d4f64b44723bf673bc2d5",
    "traceback": "viewType: field required",
    "errors": [
        {
            "loc": [
                "body",
                "viewType"
            ],
            "msg": "field required",
            "type": "value_error.missing"
        }
    ]
}

@martastain

Copy link
Copy Markdown
Member

first check if i'm right, but i think i am. then i thing the best place to do that would be here: https://github.com/ynput/ayon-backend/blob/enhancement/open_view_settings/api/views/models.py#L130

keep the existing data in the database and upon returning, check for well known keys in the json. for exampe if view type is overview and there's "sort_by", just convert to camel case. upon next save, it will be overwritten with camelCase variant and reprocessing won't be triggered again.

@martastain

Copy link
Copy Markdown
Member

this should bump minor version, since after updating, downgrading would cause a data loss @Innders

@kalisp

kalisp commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

This seems to have broken patching of views

Isn't that issue of FE though? I am now sending completely new model/content, so FE should take that into account, no? Or am I just dense (highly possible).

image

@Innders

Innders commented Jun 11, 2026

Copy link
Copy Markdown
Member

Isn't that issue of FE though? I am now sending completely new model/content, so FE should take that into account, no? Or am I just dense (highly possible).

Nothing has changed in the FE and I'm not sure why it should change?

@kalisp

kalisp commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Added optionality to view_type which got rid of the issues with validations.

@Innders Innders left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this change when we generated the queries the mutations would use payload.

Now they use genericViewPostModel and ``genericViewPatchModel`.

export type UpdateViewApiArg = {
  viewType: string
  viewId: string
  projectName?: string
  genericViewPatchModel: GenericViewPatchModel
}

can we please figure out a way so that the this goes back to payload

export type UpdateViewApiArg = {
  viewType: string
  viewId: string
  projectName?: string
  payload: GenericViewPatchModel
}

Tagging @filipvnencak

@filipvnencak filipvnencak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these backend tweaks the OpenAPI body comes through as payload again, so the frontend doesn't need the codegen-rename workaround in ynput/ayon-frontend#2066

Comment thread api/views/views.py Outdated
Comment thread api/views/views.py Outdated
Comment thread api/views/models.py Outdated
Comment thread api/views/models.py Outdated
kalisp and others added 4 commits June 22, 2026 14:33
Co-authored-by: Filip Vnenčák <vnencak.filip@gmail.com>
Co-authored-by: Filip Vnenčák <vnencak.filip@gmail.com>
Co-authored-by: Filip Vnenčák <vnencak.filip@gmail.com>
Comment thread api/views/models.py Fixed
Comment thread api/views/views.py Outdated
Co-authored-by: Filip Vnenčák <vnencak.filip@gmail.com>
@kalisp kalisp requested a review from Innders June 24, 2026 15:32
@kalisp kalisp added type: maintenance Changes to the code that don't affect product functionality (Technical debt, refactors etc.)) bump minor labels Jun 25, 2026
@kalisp

kalisp commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

this should bump minor version, since after updating, downgrading would cause a data loss @Innders

I added tag, but I am not sure if anything actually changed, format wise. I tried to set up bunch of Views and do an upgrade and content seemed same as before.

@martastain

Copy link
Copy Markdown
Member

I just reproduced the problem i was expecting. It affects fields in previously strictly typed settings that contain underscores, such as overview.show_hierarchy, overview.sort_by etc.

Steps to reproduce:

  • in develop, create an overview view that sets group_by and row_height
  • switch to this branch
  • load the view
  • group_by and row_height is not preserved.

it only does not affect filter and columns, as they don't use underscores.

possible solution:

update construct_view_model function to something like this.

def construct_view_model(**data: Any) -> ViewModel:
    view_type = data["viewType"]
    settings = data["settings"]
    if view_type in ["overview", "taskProgress", "lists", "reviews", "versions"]:
        patched_settings = {}
        for key, val in settings.items()
            if "_" in key:
                patched_settings[camelize(key)] = val
            else:
                patched_settings[key] = val
        settings = patched_settings
    return GenericViewModel(view_type=view_type, settings=settings)

@kalisp

kalisp commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

I just reproduced the problem i was expecting

F00k, didn't notice it. Implemented your comment. Added TODO to remove it in the future.

image

@martastain

Copy link
Copy Markdown
Member

Added TODO to remove it in the future.

Nothing is as permanent as a temporary workaround :-D

@martastain martastain left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works. I love when we remove code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bump minor type: maintenance Changes to the code that don't affect product functionality (Technical debt, refactors etc.))

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open up all views

4 participants