Skip to content

Integrate thumbnail: Don't change folder thumbnail#1906

Open
iLLiCiTiT wants to merge 14 commits into
developfrom
enhancement/upload-folder-thumbnails-first
Open

Integrate thumbnail: Don't change folder thumbnail#1906
iLLiCiTiT wants to merge 14 commits into
developfrom
enhancement/upload-folder-thumbnails-first

Conversation

@iLLiCiTiT

@iLLiCiTiT iLLiCiTiT commented Jun 24, 2026

Copy link
Copy Markdown
Member

Changelog Description

Do not set folder thumbnail during publishing. And unset the thumbnail on existing folders.

Additional info

Update of folder thumbnail can trigger recalculation of calculated attributes. Also setting the thumbnail id is not necessary and can be skipped.

To "auto-fix" folders that already do use the thumbnail from version was added a cleanup endpoint that does remove the thumbnail before version thumbnails are pushed to server.

Testing notes:

  1. Thumbnail integration still works and folders do have thumbnails.

@iLLiCiTiT iLLiCiTiT requested a review from BigRoy June 24, 2026 08:41
@iLLiCiTiT iLLiCiTiT self-assigned this Jun 24, 2026
@iLLiCiTiT iLLiCiTiT added the type: enhancement Improvement of existing functionality or minor addition label Jun 24, 2026
@ynbot ynbot added the size/XS label Jun 24, 2026
@iLLiCiTiT iLLiCiTiT changed the title Integrate thumbnail: Update folder thumbnail first Integrate thumbnail: Don't change folder thumbnail Jun 24, 2026

@BigRoy BigRoy 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.

Overall looks good to me - just needs the query code check and there's a minor variable name typo. @LiborBatek can you test this with regular artist user and admin user and see if it holds up fine? Run it across two different folders for both runs and they must both have had publishes with thumbnails from before this PR/branch.

Comment thread server/__init__.py Outdated
Comment thread server/__init__.py Outdated
Comment thread client/ayon_core/plugins/publish/integrate_thumbnail.py
@ynbot ynbot moved this to Review In Progress in PR reviewing Jun 30, 2026
@LiborBatek

LiborBatek commented Jul 1, 2026

Copy link
Copy Markdown
Member

Im experiencing some warning info msg when using captured thumbnail...not sure if related or irrelevant, so pls tell me @iLLiCiTiT

image

When the version does have any thumbnail it correctly get propagated from version to the folder or rather product which is good.

Now what Im not so sure is whenever producing a product version which doesnt have any thumbnail it also propagates to the product and removes any existing thumbnail too...resulting into empty thumbnail on the product. Again Im not sure if thats correct or not even tho it sounds logical to me...

Screenshot 2026-07-01 150747

So my conclusion been, if any thumb present on the version it gets correctly propagated, and if not it also propagates "nothing" and thumbnail not present.

On the Folder / Tasks all thumbnails stays intact if any existing...which is correct.

image

@LiborBatek LiborBatek 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.

I have tested using 2 distinct users (admin vs regular artist) and all seems working fine...

There are few questions from me above but I guess they are not relevant to this PR but rather how Ayon works with thumbnails/reviewables...feel free to answer those or even take those into account if connected to this PR.

Thumbnails simply stay intact when already present on a Folder and Task level and each version propagates accordingly to the product itself (whether its thumbnail or empty aka nothing).

Seems ok to me.

@github-project-automation github-project-automation Bot moved this from Review In Progress to Merge Requested in PR reviewing Jul 1, 2026
@BigRoy

BigRoy commented Jul 1, 2026

Copy link
Copy Markdown
Member

I think the above warning you mentioned actually indicates an issue - so let's make sure to double check this @iLLiCiTiT

Copilot AI 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.

Pull request overview

This PR adjusts thumbnail integration to avoid explicitly setting folder thumbnails during publishing (to prevent recalculation of calculated attributes), and adds a server-side cleanup endpoint to unset existing folder thumbnails that share the same thumbnail as a version.

Changes:

  • Update thumbnail publish integration to only set thumbnails on versions (not folders) and trigger a cleanup call for affected folders.
  • Add a core server addon POST endpoint to clear folder thumbnail_id values when they match a version thumbnail.
  • Minor logging/message adjustments around thumbnail detection and upload timing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
server/init.py Adds cleanup endpoint and SQL logic to find and unset folder thumbnails that duplicate version thumbnails.
client/ayon_core/plugins/publish/integrate_thumbnail.py Stops setting folder thumbnails, updates version thumbnails via operations session, and calls the cleanup endpoint for existing folders.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread client/ayon_core/plugins/publish/integrate_thumbnail.py
Comment thread client/ayon_core/plugins/publish/integrate_thumbnail.py Outdated
Comment thread server/__init__.py Outdated
@ynbot ynbot moved this from Merge Requested to Review In Progress in PR reviewing Jul 2, 2026

@BigRoy BigRoy 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.

Fails for me:

HTTP request error: 500 Server Error: Internal Server Error for url: http://localhost:5000/api/addons/core/1.9.7+dev/cleanupFolderThumbnails/test
{
    "status": 500,
    "detail": "TypeError: string indices must be integers, not 'str'",
    "traceback": "TypeError: string indices must be integers, not 'str'\n\nayon_server/api/context.py:77\nresponse = await call_next(request)\n\n/addons/core/1.9.7+dev/server/__init__.py:154\nfiltered_folder_ids = [row[\"id\"] for row in res]\n\n/addons/core/1.9.7+dev/server/__init__.py:154\nfiltered_folder_ids = [row[\"id\"] for row in res]"
}
ERROR   logging                    | Unhandled exception: TypeError: string indices must be integers, not 'str'
    traceback:
      TypeError: string indices must be integers, not 'str'

      ayon_server/api/context.py:77
      response = await call_next(request)

      /addons/core/1.9.7+dev/server/__init__.py:154
      filtered_folder_ids = [row["id"] for row in res]

      /addons/core/1.9.7+dev/server/__init__.py:154
      filtered_folder_ids = [row["id"] for row in res]

@github-project-automation github-project-automation Bot moved this from Review In Progress to Change Requested in PR reviewing Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS type: enhancement Improvement of existing functionality or minor addition

Projects

Status: Change Requested

Development

Successfully merging this pull request may close these issues.

5 participants