Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Settings: Flag project as deactivated and hide from tools' view #2008

Merged
merged 22 commits into from
Sep 23, 2021

Conversation

davidlatwe
Copy link
Contributor

@davidlatwe davidlatwe commented Sep 12, 2021

Feature

Able to switch project state between active and inactive in Studio Settings.

The project in inactive state will not be shown in tools like :

  • Launcher
  • Library Loader
  • Standalone Publisher
  • Project Manager
  • Local Settings
  • Sync Server

Usage

To mark project as inactive :

  1. Go to Projects tab in Studio Settings and select the project that you want to deactivate.
  2. Go to project_anatomy/attributes/active and toggle it.
  3. Save the settings

855f991b-acbd-4f33-9f9d-8590b1a453db

Tips

  • The current selected project will remain in the list view when it just got deactivated, it will be hidden when you switch to another project settings.
  • You can view all projects by checking the "Show Inactive Project" box. This check-box only exists in Studio Settings.

Notes

@davidlatwe davidlatwe added the type: feature Larger, user affecting changes and completely new things label Sep 12, 2021
@tokejepsen
Copy link
Member

Great stuff! Maybe we could have a confirmation box when using the archive switch rather than a separate line edit @iLLiCiTiT ?

class ProjectListWidget(QtWidgets.QWidget):
default = "< Default >"
project_changed = QtCore.Signal()

def __init__(self, parent):
ProjectSortRole = QtCore.Qt.UserRole + 10
Copy link
Member

Choose a reason for hiding this comment

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

Don't use came case for attributes and don't set these constants used in different code in class. It is better to define roles as constants at header part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be resolved by 0bf84ad


index = self.sourceModel().index(source_row, 0, source_parent)
is_active = bool(index.data(self.filterRole()))
is_selected = bool(index.data(ProjectListWidget.ProjectSelectedRole))
Copy link
Member

Choose a reason for hiding this comment

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

I quite don't get why selection is important? All projects should disappear when checkbox change value...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was for keeping current selected project visible in list when it just got deactivated.

Since the only way for user to know which project is being edited is from the selection of that list view, so I thought it would be better not to hide the project that just got deactivated from list as soon as the "Save" is clicked and processed, until the user changed the selection to another project settings.

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Sep 13, 2021

I would maybe change archived to active. The validation does not make much sense as you are not triggering any callbacks. It's just a flag to show/hide projects for artists plus you may accidentally "unarchive" project when you save project settings on "archived" project because confirmation won't be passed (through API). Settings are not just UI they can be (and are) use headless so validation of entered value on user base is dangerous.

BTW: The validation could be done only in UI with using custom checkbox which would trigger confirmation dialog but I don't think it's necessary.

Ad active: Also you may want to have inactive projects by default which does not make sense with having archived by default or just want pause project but not archive yet.

@mkolar
Copy link
Member

mkolar commented Sep 13, 2021

I'd also stay away from archived considering studios tend to use that as and information when project data is actually archived and not easily accessible. Active is quite nice, with it being on by default. But it also allows as @iLLiCiTiT mentioned for other workflows. For example preparing a project structure and some date before marking it as active and showing to the artists.

@davidlatwe davidlatwe changed the title Settings: Flag project as archived and hide from tools' view Settings: Flag project as deactivated and hide from tools' view Sep 15, 2021
@davidlatwe
Copy link
Contributor Author

davidlatwe commented Sep 15, 2021

All good points, I have changed the attrib archive to active.

It's just a flag to show/hide projects for artists plus you may accidentally "unarchive" project when you save project settings on "archived" project because confirmation won't be passed (through API).

That's true. I removed the confirmation for now as it's also outdated after the archive attrib is changed.

But after the confirmation is removed, code like this (e.g. in avalon-core code base AvalonMongoDB.projects) may not work as expect if the default setting active is changed to false :

for doc in project_docs:
    if not doc["data"].get("active", True):
        # the default value here is not following studio default setting
        continue
    ...

Not sure how to solve this properly, as it may also make sense if one like the project's default active state to False ?

For new project, we could make the project manager to always set the data.active in database with studio default when creating new one. But for the existing old projects, how do we handle that ?

Edit

But for the existing old projects, how do we handle that ?

How about scan and update project documents that doesn't have data.active and set it to true on pype tray launch ?

@iLLiCiTiT
Copy link
Member

But after the confirmation is removed, code like this (e.g. in avalon-core code base AvalonMongoDB.projects) may not work as expect if the default setting active is changed to false

I don't think it's used in a way that it would change behavior or affect anything.

But for the existing old projects, how do we handle that ?

Since there is (in avalon-core) default value of "active" set to True (project_doc["data"].get("active", True)) it should be fine as all projects without the key are proposed as active.

How about scan and update project documents that doesn't have data.active and set it to true on pype tray launch ?

Not in tray like a script before/after launch that's not good idea as it would slow down all tray applications if studio has 30 projects it would query 30 times on each tray launch without doing anything. It could be done by script (out of tray), through settings or could modify AvalonMongoDB.projects to set the key to True if is not set or value is None. The last one is most easiest but is forcing to have that key on project...

@davidlatwe
Copy link
Contributor Author

davidlatwe commented Sep 15, 2021

Since there is (in avalon-core) default value of "active" set to True (project_doc["data"].get("active", True)) it should be fine as all projects without the key are proposed as active.

Yeah, but if the default value in Studio Settings gets change to false, the code won't be align with the settings (for existing projects that without the key). Or this behavior should be considered as edge case ?

…ema_anatomy_attributes.json

Co-authored-by: Milan Kolar <mkolar@users.noreply.github.com>
@mkolar mkolar self-requested a review September 16, 2021 08:09
Copy link
Member

@mkolar mkolar left a comment

Choose a reason for hiding this comment

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

Actually I spoke too soon. Sync queue is showing no projects at all with this PR. @davidlatwe get in touch with @kalisp he should be able to point to to a simple site sync setup with local storage so you can sort it out.

if only_active:
inactive_chk = None
else:
inactive_chk = QtWidgets.QCheckBox(" Show Inactive Projects ")
Copy link
Member

Choose a reason for hiding this comment

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

Use parenting on widgets for proper style propagation.

Suggested change
inactive_chk = QtWidgets.QCheckBox(" Show Inactive Projects ")
inactive_chk = QtWidgets.QCheckBox(" Show Inactive Projects ", self)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I tried this but doesn't have any difference.

Copy link
Member

Choose a reason for hiding this comment

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

I know but it may differ if we had different styles for checkbox than default.

@mkolar mkolar self-requested a review September 16, 2021 17:30
@davidlatwe
Copy link
Contributor Author

davidlatwe commented Sep 16, 2021

Thanks @iLLiCiTiT for taking a stab on fixing SyncServer ❤️

However I found an issue :

  1. I deactivate a project
  2. open up SyncServer and find that inactive project still showing in the list
  3. select that inactive project in SyncServer
  4. entire Pype tray crashed

As for Launcher, LibraryLoader, StandalonePublisher, LocalSettings and ProjectManager, they all seems fine in a few quick tests.

@mkolar
Copy link
Member

mkolar commented Sep 16, 2021

However I found an issue :

Oh good testing. I literally just tested too and didn't notice that

@mkolar
Copy link
Member

mkolar commented Sep 16, 2021

But I can confirm that it indeed crashes in the scenario that @davidlatwe described

@iLLiCiTiT
Copy link
Member

So in the end @kalisp will have to look into that :)

@davidlatwe
Copy link
Contributor Author

Here's the full log and GIF that shows the bug
davidlatwe#1 (comment)

@mkolar
Copy link
Member

mkolar commented Sep 20, 2021

@kalisp
Copy link
Member

kalisp commented Sep 22, 2021

Fixed Site Sync issue in davidlatwe#2

@mkolar mkolar added type: enhancement Enhancements to existing functionality type: feature Larger, user affecting changes and completely new things and removed type: feature Larger, user affecting changes and completely new things labels Sep 23, 2021
@mkolar mkolar merged commit bf02118 into ynput:develop Sep 23, 2021
@mkolar mkolar removed the type: feature Larger, user affecting changes and completely new things label Sep 23, 2021
@mkolar
Copy link
Member

mkolar commented Sep 23, 2021

@kalisp @davidlatwe apologies. I've prematurely merged this thinking the fix was already part of it here.

Petr could you please make the same PR to this main repo and we'll make a hotfix asap with it. Thanks.

@davidlatwe davidlatwe deleted the feature/project-archive-state branch September 23, 2021 18:15
@mkolar mkolar added type: feature Larger, user affecting changes and completely new things and removed type: enhancement Enhancements to existing functionality labels Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Larger, user affecting changes and completely new things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants