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

Generation / Executions UI with many data sources #2776 #2789

Merged
merged 3 commits into from
Jan 25, 2023

Conversation

anton-abushkevich
Copy link
Contributor

Resolves #2776

  1. All Generation/Execution sources lists now work as datatable:
    -- user can filter sources by name
    -- datatables display items (sources) count and add pagination when necessary
  2. Added checkbox to hide sources without generation/execution results
  3. Two types of generation/execution tables
    -- for Characterizations, Pathways, PLP and PLE (with results versioning)
    -- for Cohorts and Incidence Rates (without versioning, displaying results in the table)

Copy link
Collaborator

@chrisknoll chrisknoll left a comment

Choose a reason for hiding this comment

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

I noticed the use of subscriptions, but I'd like to remove subscriptions in favor of pureComputed (subscriptions will need to be disposed of, and the 'flow' is more logical to use computed instead of responding to subscriptions. here is how:

Define a prueComputed to yield the filteredExecutionGroups, something like:

  this.filteredExecutionGroups = ko.pureComputed(() => {
    return (this.showOnlySourcesWithResults() ? 
                  this.executionGroups().filter(eg => eg.submissions().length > 0)
                  :  this.executionGroups());
  });

In this way, the pureComputed re-calculates automatically if the checkbox changes (the showOnlySourcesWithResults) or if the executionGroups() changes. Note, may need to add logic that calls this.executionGroups.valueHasMutated() if there is a background process that refreshes the execution groups when jobs are running.

Since your datatables are being bound to filteredExecutionGroups, that code won't need to be changed if filteredExecutionGruops becomes a computed observable.

@anton-abushkevich
Copy link
Contributor Author

@chrisknoll
Subscription is replaced with pureComputed as you suggested - please review: 1a888df

Copy link
Collaborator

@chrisknoll chrisknoll left a comment

Choose a reason for hiding this comment

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

Great PR: the changes were made exactly where this feature request would expect them to be, it's following good coding conventions, and is a great feature with minimal configuration or administrative burden. Good job.

@chrisknoll chrisknoll merged commit 9649e95 into master Jan 25, 2023
@delete-merged-branch delete-merged-branch bot deleted the generation_execution_ui_rework branch January 25, 2023 16:38
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.

Generation / Executions UI with many data sources
2 participants