search

getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Handle case of asset having no tasks #434

Closed mottosso closed 5 years ago

mottosso commented 5 years ago

Problem

With #403 merged, a user may find himself without ability to select a task, nor any indication of why or how to address it.

Solution

Either tell the user, or give the user an option to assign a task from the Context Manager.

image

davidlatwe commented 5 years ago

This might be related to the comment from @BigRoy here and here, because it's also letting people have opportunity to write management related stuff into database.

Currently no authentication related issue yet, maybe it's a good time ?

mottosso commented 5 years ago

That's a good point.

Let's make this issue about telling the user that no task has been assigned (rather than actually assigning any), and that one can be created via the Project Manager. Then we can delegate authentication (in whatever form) to there.

mottosso commented 5 years ago

image

I've added this, but there's a large amount of technical debt in the Context Manager.

  1. Models shared cross-referenced across tools, projectmanager and contextmanager
  2. Models are too specialised; TasksModel. The result is overly specific methods, like set_assets. Should be generalised, and IO centralised in io.py
  3. Tree model for what is a flat list
  4. Unique naming convention for __default__, should be default
  5. Unique naming convention for COLUMNS, should be Columns
  6. Unique naming convention for "node", should be "item" (these are "QAbstractItemModels")
  7. Exceptions thrown to views as assertion errors
  8. Exceptions not actually handled by views

The list goes on.. this affects #412 too. Wouldn't bother repairing these, the combined Context Manager and Workfiler should be a new project altogether.

BigRoy commented 5 years ago

Exceptions not actually handled by views

Could you describe how one would do that? How can the view capture the errors of the model, code-wise that is. Do you have an example?

Models are too specialised; TasksModel. The result is overly specific methods, like set_assets. Should be generalised, and IO centralised in io.py

Could you elaborate on how you feel this should be centralised to io.py? I'm not too sure I'm really feeling what's wrong with using the assets for a model that displays the tasks of assets.

Tree model for what is a flat list

I believe this was to support multiple columns. I recall the list not supporting it. Basically the project manager allows to also list the amount a specific tasks is present (when you select multiple assets at the same time). We use that quite a bit to easily figure out if one task might be missing one one or more assets among 10s of assets.

Unique naming convention for default, should be default

This would conflict if someone had a family they called "default". Keep that in mind.

The cross-referencing is part of what the gui PR would have resolved. I think you will hit that problem for almost all tools and if we are cleaning up I'd prioritise doing what that PR intended to do... but then the right way as you'd like to see it. It will be a big PR because it will refactor a lot of code. :)

Also, subjective cosmetic note... shouldn't it be "No tasks" as opposed to "No task". It almost sounds as if only a single task could be present. Also, should it be italic or something to show that it's special?

mottosso commented 5 years ago

Sorry, those should really have been marked "Note to self", was typing too quickly there.