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

Loader tool: Fix filtering #783

Merged
merged 8 commits into from
Jul 18, 2024
Merged

Loader tool: Fix filtering #783

merged 8 commits into from
Jul 18, 2024

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Jul 17, 2024

Changelog Description

Loader tool can enable/disable grouping of products and filtering by product types and status names should work.

Additional info

This was broken with new statuses filter which required to pass status names on model refresh, which was mid-solution that is not necessary at the end. So changed the logic of Products model refresh back to previous integration to not require status names on refresh.
PR changed during it's lifetime as was discovered that the whole filtering was not working. Product type filtering was completelly broken and statuses filtering did break UI for multiselection of folders. Also products under group item did not propagate version change from combobox in specific cases.

Testing notes:

  • First of all grouping can be enabled/disabled.
  • To validate we didn't break the source of the previous implementation:
  1. Open loader tool.
  2. Add some status filters (that will change version to lower version than latest).
  3. Refresh or enable/disable grouping.
  4. The version should stay at same version number and should show correct status and rest of version related information.

Resolves #782

@iLLiCiTiT iLLiCiTiT marked this pull request as ready for review July 17, 2024 15:54
@ynbot ynbot added type: bug Something isn't working size/XS labels Jul 17, 2024
@iLLiCiTiT iLLiCiTiT requested a review from BigRoy July 17, 2024 15:54
@BigRoy
Copy link
Collaborator

BigRoy commented Jul 17, 2024

So first off, the grouping works. I can now enable/disable it just fine.

However, I did get this error with multi folder selections:

Traceback (most recent call last):
  File "E:\dev\ayon-core\client\ayon_core\tools\loader\ui\products_widget.py", line 71, in filterAcceptsRow
    if not self._accept_row_by_statuses(index):
  File "E:\dev\ayon-core\client\ayon_core\tools\loader\ui\products_widget.py", line 82, in _accept_row_by_statuses
    for status in status_s.split("|"):
AttributeError: 'NoneType' object has no attribute 'split'

It's only filtering to the "First status" inside a Group... not the individual statuses of each entry in the group.

image
image

Note how these are displaying statuses that are not selected. I suspect this is not a new issue due to this PR though?


Similarly the filtering gets confused it seems with Multiselections:

image

I think in this case the filter is correct, but...
image

It's just not displaying the status correctly maybe?


Status filter finds 'last matching version' instead of filtering current list

Here I have no filters visible, sort by status... there are three approved entries:

image

I set the filter to "Approved" statuses only and suddenly get:

image

Is the behavior correct that when filtering to a particular status that it shows the last matching version with that status. So if e.g. an OLDER version was approved it'd then display that instead?

The same occurs when picking e.g. Omitted:

image

But if I don't set that filter it lists much newer versions that are not omitted at all.

Is this correct behavior or confusing? :)
I was confused at first.


This isn't a critical issue but it does look confusing. Say I have a filter:

When disabling the filter without going out of the dropdown menu the "version" of the product changes (likely due to the filtering described above). However, the status and the rest of the row does not update until I go out of the dropdown menu.

image
image
image

@iLLiCiTiT
Copy link
Member Author

Didn't read fully (looong text), but can confirm it is not intentional and should be fixed, could you try if the issue with groups (multiselection of folders) also occurs in develop?

@BigRoy
Copy link
Collaborator

BigRoy commented Jul 17, 2024

Didn't read fully (looong text), but can confirm it is not intentional and should be fixed, could you try if the issue with groups (multiselection of folders) also occurs in develop?

Yes. Also occurs using develop.

So basically - this PR fixes the grouping. And we can track this as a separate issue instead.

Single folder select, status filtering fails in groups:
image

BUT I think it's actually correctly filtering to the last "Approved" version of that product however it fails to display its status correctly. Because if I manually go to v029 for workfileModeling I get:

image

Hence, the filtering may just be correct (it is actually approved).


Here with multiselect folder - similar issue:

image

It's the same product even! :)

@iLLiCiTiT iLLiCiTiT changed the title Loader tool: Fix grouping with status filtering Loader tool: Fix filtering Jul 18, 2024
Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

I can't seem to break the filtering now. 👍

It works

So, works as intended.


I do still think that the filter by status finding the "latest version" that matches of a product instead of actually filtering the list is a bit confusing behavior. I wouldn't have expected it and I wonder whether we need a tooltip explaining that AND/OR even have a toggle to disable that. It disallows me to find the "approved" versions that are currently listed (those that are the latest version).

I'll create another issue for that.

Also, it only dawned on me now that it ALSO filters the version selection boxes of the products 🤯 - it's actually quite nice, but still unexpected for me.


And a minor cosmetic one is that if it does filter out e.g. two products when multiselecting three folders that have a product by that name is that the "header" group of the multiselection will still list the number (3) at the end even though only one is listed.

image

This is hardly an issue - but just wanted to point that out.

@iLLiCiTiT iLLiCiTiT merged commit 564365a into develop Jul 18, 2024
3 checks passed
@iLLiCiTiT iLLiCiTiT deleted the bugfix/loader-tool-grouping branch July 18, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loader: "Enable grouping" toggle fails due to invalid access to statuses
3 participants