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

Improve management of tasks within the client #233

Merged
merged 20 commits into from
Jul 4, 2023

Conversation

rkpasia
Copy link
Contributor

@rkpasia rkpasia commented Jun 28, 2023

The user should be able to navigate through the tasks in the client with more context.

That is:

  • Provide better sorting of tasks in tasks page.
  • Provide better filtering of tasks in workflow-building page.

Merging this PR should close #149

@tcompa
Copy link
Collaborator

tcompa commented Jun 28, 2023

Current status of the tasks page already looks good:

Screenshot from 2023-06-28 12-11-37

I can review more in detail later, especially if this has to evolve a bit more.

(minor decision: should versions be decreasing or increasing?)

@tcompa
Copy link
Collaborator

tcompa commented Jun 28, 2023

@rkpasia note that I included multiple versions of the same package, for testing - see e9322ea

@rkpasia
Copy link
Contributor Author

rkpasia commented Jun 28, 2023

Great! I also tested by editing the database entries 👌🏼

- Select task group
- Select filtered tasks
- Select version
- Extracted sorting function
- Improve usage of semver to handle PEP440 versioning
- Introduce testing of sorting versioning
@tcompa
Copy link
Collaborator

tcompa commented Jun 30, 2023

Testing with 232dd05, and also the order of prerelease versions is now correct:

Screenshot from 2023-06-30 12-41-36

@tcompa
Copy link
Collaborator

tcompa commented Jun 30, 2023

For the record, @rkpasia introduced the modal to add a task to a workflow as a placeholder. This is not (yet) meant to be working/testable.

@tcompa tcompa marked this pull request as draft June 30, 2023 11:30
@jluethi
Copy link
Collaborator

jluethi commented Jun 30, 2023

Looking promising. Ping me once you want me to test this as well :)

@rkpasia rkpasia requested review from jluethi and tcompa June 30, 2023 14:22
@jluethi
Copy link
Collaborator

jluethi commented Jun 30, 2023

😍 @rkpasia the new task selector is awesome! That makes it much more structured, easy to keep an overview!

The task ordering seems to work as I'd expect it and it selects the 0.10.0 over 0.10.0a6 correctly!

Screenshot 2023-06-30 at 18 02 59

The split into the tabs for common tasks & User tasks also looks nice! I haven't tested it in the multi-user case yet, will follow up on that in a bit.


An important thing that I'd still add now would be how to handle custom tasks without (unique) versions. If one adds a task via the web interface, so far the user can't specify a version (we should eventually also fix that, but not so urgent). It then gets displayed as follows (2 custom tasks with the same name, but different source):

Screenshot 2023-06-30 at 18 03 31

That's quite suboptimal, as there's no way to know which one is which.
This would also happen if the user provides a version, but that version is not unique (we're currently not enforcing unique versions). What we are enforcing are unique sources. Thus, let's proceed as suggested here:
#149 (comment)

If a given user has multiple tasks with the same name, we show a drop-down.
There, following your logic:

If a group of users' tasks have same name+owner and they all have different versions, proceed as for common tasks (i.e. display them as grouped).
If the versions are not unique, we could do dropdown by source. Alternatively, we just always to dropdown by source for user specific tasks with multiple tasks of the same name. Now source is not as elegant as version of course, but it will always be unique.

@jluethi
Copy link
Collaborator

jluethi commented Jun 30, 2023

We could also still improve the multi-user display a bit. At the moment, it just puts them all in one big list and there is no way to see which user a task is from. I don't fully know how that list is sorted (also by when the task was added?).

Here's an example: I created tasks by 3 users, some named the same, some with custom names. It is shown like this:

Screenshot 2023-06-30 at 17 48 41

What I'd like to achieve:

  1. Have an easy way for users to find their own tasks
  2. Have an easy way for a user to find a task of another given user (i.e. Tommaso tells me he made a task to solve my issue, I should know hot to find that)

But: Let's NOT just have a user dropdown first to filter by user. Because then, the user needs to look through all the user dropdowns just to find the task they know by name.

One idea here from @tcompa :

A group of tasks that have the same task.name, the same task.owner and a different task.version are to be considered (as the name says) different versions of the same tasks, and shown as a single group (with the latest version pre-selected).

#149 (comment)

We could order the user task list in the following way:
Logged-in user comes top, afterwards alphabetical by task.owner [=> username / slurm_user (users need to have one of those 2 set to register custom tasks)].
And the task would be shown as follows (if logged in as jluethi):
jluethi: Joel_task
jluethi: TestTask
admin: TestTask
joel_2: Joel_2_Task

But if one is logged in as admin, one sees:
admin: TestTask
jluethi: Joel_task
jluethi: TestTask
joel_2: Joel_2_Task

@jluethi
Copy link
Collaborator

jluethi commented Jun 30, 2023

And a word regarding ordering of common task: Ordering by package should be the goal. I think the current sorting achieves this. But if we change sorting, let's explicitly order the common task by their package :)
In a best case, we'd even show the package name (same as the username for custom ones) in some fancy way, i.e. a small box after the task name. But prepending it to every common task, i.e. having them as
fractal-tasks-core: Create OME Zarr

would not be a desired outcome.

Alternatively, is it possible to have non-selectable "headers" in the dropdown? For packages in the common case, for username in the other case. Something functionally like this:
Vt94a

# Conflicts:
#	package-lock.json
#	package.json
- Common tasks gets grouped in select by their reference package
- User tasks gets grouped by their owners
@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

In the changes introduced, 54a0a10 enables

We could order the user task list in the following way:
Logged-in user comes top, afterwards alphabetical by task.owner [=> username / slurm_user (users need to have one of those 2 set to register custom tasks)].
And the task would be shown as follows (if logged in as jluethi):
jluethi: Joel_task
jluethi: TestTask
admin: TestTask
joel_2: Joel_2_Task
But if one is logged in as admin, one sees:
admin: TestTask
jluethi: Joel_task
jluethi: TestTask
joel_2: Joel_2_Task

14d204d introduces a project dependency that I selected from npm which is slim-select.
This library enables us to group and label many tasks together specifying how we want them to be grouped by as suggested

And a word regarding ordering of common task: Ordering by package should be the goal. I think the current sorting achieves this. But if we change sorting, let's explicitly order the common task by their package :)

Alternatively, is it possible to have non-selectable "headers" in the dropdown? For packages in the common case, for username in the other case.

Screenshot 2023-07-04 alle 08 27 13
Screenshot 2023-07-04 alle 08 27 02

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

@rkpasia
The new layout for common task per package in the dropdown is great, sorting of versions works as expected. Let's go with that!

The sorting of tasks by users also looks great. Can we take always put the currently logged in user top or does that get too complicated? Current sorting is ok, this sorting with logged in user at the top (if they have tasks) would be even better

Search is not functional yet, right? But great idea, would love search when adding the tasks!

And for multiple user tasks with the same name, but no version, we still get the dropdown of all "Not specified" => if that would be replaced by the source in 2 cases, then it would work best.
The 2 cases are:

  1. No version is present (currently gets replaced with "Not specified")
  2. Version is not unique for that user & task

@tcompa
Copy link
Collaborator

tcompa commented Jul 4, 2023

I confirm that the behavior for common tasks looks reasonable to me:

Screenshot from 2023-07-04 10-35-54

Screenshot from 2023-07-04 10-36-02

@tcompa
Copy link
Collaborator

tcompa commented Jul 4, 2023

Search is not functional yet, right? But great idea, would love search when adding the tasks!

Agreed, that would be a nice idea. What would we be searching though? Task name?
Anyway, if it's not currently functional let's just remove it for the moment, and we can enable it later.

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

If it's a trivial part of the framework to allow search by task name, that would make things much simpler once there are many task.
i.e. I named my task "custom_measurement_joel", I don't see it in the list right away, so I start typing:
custom... => only see the task where the names contain the word custom.

Any fancier search can be postponed without issues. And if it's tricky to get to the core search pattern, we can also postpone that (and thus remove the field for the moment).

@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

I still have to read through the documentation of slim-select to understand how the search box works.

I think that in principle should be useful to search by task name. Don't know if it could be possible to also introduce a filtering by specifying, for instance, the package name or the package version. Maybe applying a custom search function could be doable.

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

I think that in principle should be useful to search by task name.

Agreed, this will be very useful!

Don't know if it could be possible to also introduce a filtering by specifying, for instance, the package name or the package version. Maybe applying a custom search function could be doable.

Given the need for documentation and testing for Fractal web, I'd say a short effort on search by task name is still in scope, custom search functions to include package names, versions etc. is out of scope (though a good feature idea for future development).

@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

The sorting of tasks by users also looks great. Can we take always put the currently logged in user top or does that get too complicated? Current sorting is ok, this sorting with logged in user at the top (if they have tasks) would be even better

Has been introduced by 1e5f7e1

In the following image, my username is test and have a task that has test as owner.
Other tasks follow sorting by owner, name.

Screenshot 2023-07-04 alle 11 15 13

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

Awesome, looks great like this.
Does it still work for users that don't have custom tasks (i.e. they'd just see admin first)?

@tcompa
Copy link
Collaborator

tcompa commented Jul 4, 2023

There are still use cases which are not covered (see also #233 (comment) by @jluethi).

I create the following tasks

name="test", owner="admin", source="admin:a", version=null
name="test", owner="admin", source="admin:b", version=null
name="test", owner="user", source="user:a", version=null

and they are correctly displayed as

admin
    test
user
   test

Then I click on the first test task name (i.e. the one belonging to admin), and I'm shown

 Select task version 
    Not specified
    Not specified
    Not specified

We are hitting two issues here:

  1. If we are separating admin/test and user/test, then "Select task version" should only show two items (the ones with owner=admin) rather than three.
  2. If the version is not there, this selection is not meaningful (independently on point 1). I think that "Not specified" should always be replaced by the task source, and "Select task version" should be "Select task version/source".

When selecting a task by owner, only task versions of that owner should be shown in version selection.
@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

Agreed, these two things are still missing.

  1. If we are separating admin/test and user/test, then "Select task version" should only show two items (the ones with owner=admin) rather than three.
  2. If the version is not there, this selection is not meaningful (independently on point 1). I think that "Not specified" should always be replaced by the task source, and "Select task version" should be "Select task version/source".

In addition:

Search is not functional yet, right? But great idea, would love search when adding the tasks!

I'd see these 3 areas as the last things remaining here

@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

@tcompa I noticed this bug

If we are separating admin/test and user/test, then "Select task version" should only show two items (the ones with owner=admin) rather than three.

And made a fix in 5a44460.

Now it should work.

Related to

If the version is not there, this selection is not meaningful (independently on point 1). I think that "Not specified" should always be replaced by the task source, and "Select task version" should be "Select task version/source".

Now if the version is not specified, the source property is shown. Moreover, if there are multiple tasks with the same version, only source should be displayed in the task version/source selection.

@tcompa
Copy link
Collaborator

tcompa commented Jul 4, 2023

Moreover, if there are multiple tasks with the same version, only source should be displayed in the task version/source selection.

Is this already implemented?

I have tasks with versions "1", "2", and "2", and in the drop-down menu I see

  • v1
  • v2
  • user:3 (which is the source of the third task)

I don't have a strong opinion here, but I would be OK with your idea (if there are multiple tasks with the same version, only source should be displayed in the task version/source selection) - once it's implemented.

@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

If the version is not there, this selection is not meaningful (independently on point 1). I think that "Not specified" should always be replaced by the task source, and "Select task version" should be "Select task version/source".

On this we could go even further, given:

  • A user want to select a task to insert in the workflow
  • Opens the new selection tab
  • Selects the Users tasks tab
  • Now, if a user selects a task that has multiple same versions in the server, but they are assigned different owners, in the select version/source dropdown the user should be shown just the version name and not the source name.

Currently, the client, if multiple same task versions are present, they are replaced with source name instead of version, by default.
Should we discriminate by user?

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

if there are multiple tasks with the same version, only source should be displayed in the task version/source selection

I'd agree with that as well

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

Now, if a user selects a task that has multiple same versions in the server, but they are assigned different owners, in the select version/source dropdown the user should be shown just the version name and not the source name.
Currently, the client, if multiple same task versions are present, they are replaced with source name instead of version, by default.
Should we discriminate by user?

If we easily can, then yes.

If a user selects a task within a package, only task versions related to that task package should be used in the afterward select as options.
If a user selects a task owned by a user which has multiple same versions, selection proceeds using source property instead of version property
@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

Ok, so a quick recap:

  • Task can be picked by two groups, common and user
  • Common tasks are again grouped by package
    • When selecting a task that belongs to a package, if there are multiple tasks with same name and version, source property is used to select the task instead of version.
  • Users tasks are grouped by owner
    • When selecting a task that belongs to a user, if there are multiple tasks with same name and version, source is used instead of version. (Same as common group)
  • The select box groups tasks automatically through headers
  • We should include the search feature to improve further the selection experience once there will be many tasks available.

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

  • Task can be picked by two groups, common and user
  • Common tasks are again grouped by package
  • When selecting a task that belongs to a package, if there are multiple tasks with same name and version, source property is used to select the task instead of version.
  • Users tasks are grouped by owner
  • When selecting a task that belongs to a user, if there are multiple tasks with same name and version, source is used instead of version. (Same as common group)
  • The select box groups tasks automatically through headers
  • We should include the search feature to improve further the selection experience once there will be many tasks available.

Agreed with all of this, this is the behavior we want. Great summary!

then "Select task version" should only show two items (the ones with owner=admin) rather than three

i.e. only the version or source for the task belonging to a given user should be shown in the dropdown, not all all available versions for this task among the users.

@tcompa
Copy link
Collaborator

tcompa commented Jul 4, 2023

When selecting a task that belongs to a package, if there are multiple tasks with same name and version, source property is used to select the task instead of version.

I suggested this feature for cases where we do include additional requirements for task collection (e.g. the python version, or some additional extras). This could generate tasks that have the same (package, name, version), and then we should use the source to identify them.
Note that implementing a full parsing of the source (that also reads the python version and/or the extras) doesn't seem very future-proof. Also, I don't expect that explicitly setting the python version is something we are going to need often.

@jluethi
Copy link
Collaborator

jluethi commented Jul 4, 2023

I suggested this feature for cases where we do include additional requirements for task collection (e.g. the python version, or some additional extras). This could generate tasks that have the same (package, name, version), and then we should use the source to identify them.

This sounds future-proof enough to me:
In most cases, name + version are unique, thus show them.
In the special cases where they are not, show the source (not as pretty, but guaranteed to be unique)

@rkpasia
Copy link
Contributor Author

rkpasia commented Jul 4, 2023

We should include the search feature to improve further the selection experience once there will be many tasks available.

Has a related problem #238

@rkpasia rkpasia marked this pull request as ready for review July 4, 2023 13:53
Copy link
Collaborator

@jluethi jluethi left a comment

Choose a reason for hiding this comment

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

I tested the most recent version. All works as desired. Great job @rkpasia ! 🎉

I'd say ready to merge and we have search functionality as a future feature wish (not critical atm).

@rkpasia rkpasia merged commit 3f93162 into main Jul 4, 2023
2 checks passed
@tcompa tcompa deleted the display-task-list-with-richer-structure branch July 5, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display task list with richer structure
3 participants