86 pagination #97

ri-pandey · 2023-10-18T21:03:38Z

Description

Added pagination to Dataset table (Raw Data/Data Product) pages.
Pagination works using OFFSET and LIMIT clauses in the SQL.
Pagination respects any search/filter/sort criteria specified. Results are fetches in batches based on the criteria specified.

Related Issue(s)

Closes #86

Changes Made

Feature added
Code refactored
Documentation updated
Other changes:
Modified HTTP endpoint used for retrieving datasets to enable sorting/searching/filtering queries to be run across the entire Dataset table, and to limit and offset the datasets retrieved. All new criteria are optional. Also added ability to sort by fields in the metadata column, with some reasonable defaults.
Disabled sorting on 'Archived' and 'Staged' fields, and made those filters instead.

Checklist

Before submitting this PR, please make sure that:

Your code passes linting and coding style checks.
Documentation has been updated to reflect the changes.
You have reviewed your own code and resolved any merge conflicts.
You have requested a review from at least one team member.
Any relevant issue(s) have been linked to this PR.

added basic Pagination to DatasetList added .idea to .gitignore added basic search functionality retain search criteria when paginating across filtered results made offset computed property of currentPageIndex WIP - building framework for sorting renamed var enabled sorting by columns in Dataset table changed comments account for no sorting allow for sort criteria to not be included in request added comment fixed comments retrieve updated results by query only re-trigger result retrieval when sort query changes visually indicate default sort field; sort by one field at a time update page count whenever results are retrieved visually indicate default sorting field on page load; update source refs instead of computed property when sort criteria changes watch computed properties instead of source refs edited comments edited comments edited comments edited comments minor refactoring made pagination part of table enabled sorting datasets by num_genome_files field added filters for Staged and Archived fields retain filter criteria across pages

deepakduggirala · 2023-10-18T21:08:08Z

.gitignore

db/postgres has scripts/install_extensions.sh, a script to install pg extensions. Could you please specify why this directory is ignored?

I didn't add the ignore. My IDE simply added a newline character after the last line.

I believe that I added this ignore a while ago to prevent database data files from being included when developing locally with docker. Now that we're using prisma migrations, we may need to adjust this rule.

I see, I'll update it. Rishi, you can ignore this.

deepakduggirala · 2023-10-18T21:26:05Z

api/src/routes/datasets.js

+    query('type').isIn(config.dataset_types).optional(),
+    query('name').notEmpty().escape().optional(),
+    query('days_since_last_staged').isInt().toInt().optional(),
+    query('limit').isInt().toInt().optional(),


prisma query may fail when limit / offset are null/undefined

The UI service method which calls this endpoint will simply omit any null / undefined fields.

I tried explicitly sending limit=null in my URL, in which case the response complains about invalid value being supplied to limit, since we have isInt() validation defined on it. As far as I can tell, this is what we want?

Did you have any other scenario in mind?

No, you are correct that the validator will throw error if the request has null values.

FYI, along with the UI, this API is also used by the workers.

The workers' watch script uses this API to get the list of names of all registered datasets. So, it would be simpler to return all results when limit and offset are not provided which I guess this current design does.

deepakduggirala · 2023-10-18T21:33:19Z

api/src/routes/datasets.js

@@ -22,6 +22,80 @@ const isPermittedTo = accessControl('datasets');
 const router = express.Router();
 const prisma = new PrismaClient();

+const default_metadata_field_comparison_fns = {


can you use this instead?

function cmp(a, b) { return a != null && b != null ? (a < b ? -1 : a > b ? 1 : 0) : 0; }

deepakduggirala · 2023-10-18T21:35:11Z

api/src/routes/datasets.js

@@ -176,6 +329,17 @@ router.get(
      },
    });

+    // Perform sorting by metadata fields, if said fields are included in request.


Sorting after filtering and pagination will not yield correct results.

Drop the feature of sorting datasets by metadata fields, this would reduce the code complexity.

deepakduggirala · 2023-10-18T21:41:48Z

api/src/routes/datasets.js

 router.get(
-  '/',
+  '/count',


It would be beneficial to consider incorporating the count within the /datasets endpoint rather than having it as a distinct endpoint. This adjustment is not just about code organization and aesthetics; it has practical implications. When the count is separated from the data endpoint, there is potential for variations in results when different users attempt to add, update, or delete datasets.

One approach to address this concern is to encapsulate the count Prisma function and the data function within a transaction. The response to the /datasets endpoint could take the following form:

{ "metadata": { "total": 50 }, "datasets": [] }

TODO: By changing API response format, we need to update the code that consumes this API.

Updated this for the UI side.

deepakduggirala · 2023-10-18T21:55:38Z

While sorting in descending order by size, the datasets which do not have any size are appearing on top. Desired: they should be at last (undefined / null is less than 0 in this case)

working with simplified compare fn further simplified sort function removed sorting by num_genome_files

ri-pandey · 2023-10-19T16:54:18Z

While sorting in descending order by size, the datasets which do not have any size are appearing on top. Desired: they should be at last (undefined / null is less than 0 in this case)

I have fixed this as well. Turns out that Prisma does not support sorting by the nulls-last option for non-nullable columns. It also does not support sorting by multiple columns when some of them are nullable, and others are not. Therefore, to enable sorting in both nullable-field and non-nullable-field cases, I had to limit the sortBy to a single column.

deepakduggirala · 2023-10-19T16:58:58Z

api/src/routes/datasets.js

-        },
-      };
-    }
-
    const datasets = await prisma.dataset.findMany({


get total count and results in one transaction. see https://stackoverflow.com/a/74334140

deepakduggirala · 2023-10-19T17:01:40Z

ui/src/components/dataset/DatasetList.vue

@@ -219,7 +260,7 @@ const columns = ref([
    label: "archived",
    thAlign: "center",
    tdAlign: "center",
-    sortable: true,
+    sortingFn: () => {}, // overrides va-data-table's default sorting behavior


do we need sortingFn on columns where sortable is false (default)?

…in workers

ri-pandey added 12 commits October 18, 2023 16:19

update count by filter selection

6489f9a

got sorting working with updated conditions

80dc522

simplified conditions

94f8479

added ability to use default comparison fns, or custom ones

8d1f777

place elements without a value after the ones with a value while sorting

8ca4c8d

renaming

68d91cd

moved method outside endpoint block

57268f3

cleanup

7b24547

hide pagination if there aren't sufficient results

9c493d8

cleanup

7b208d7

added comments

1a193f6

ri-pandey requested a review from deepakduggirala October 18, 2023 21:04

deepakduggirala reviewed Oct 18, 2023

View reviewed changes

ri-pandey added 3 commits October 19, 2023 10:51

Removed ability to sort by num_genome_files

125ae73

working with simplified compare fn further simplified sort function removed sorting by num_genome_files

reuse existing endpoint to retrieve count

71e81c1

enable sorting nullable columns by nulls last or first

c60baf6

deepakduggirala reviewed Oct 19, 2023

View reviewed changes

ri-pandey and others added 3 commits October 19, 2023 13:54

retrieve count and datasets in one transaction

6963a99

remove sortingFn where not needed

e8ae394

center pagination; avoid duplicate xhr calls, update api client code …

a87e63b

…in workers

deepakduggirala approved these changes Oct 19, 2023

View reviewed changes

deepakduggirala merged commit f2c4981 into main Oct 19, 2023

deepakduggirala deleted the 86-pagination branch October 19, 2023 19:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

86 pagination #97

86 pagination #97

ri-pandey commented Oct 18, 2023 •

edited

Loading

deepakduggirala Oct 18, 2023

ri-pandey Oct 19, 2023

charlesbrandt Oct 19, 2023

deepakduggirala Oct 19, 2023

deepakduggirala Oct 18, 2023

ri-pandey Oct 19, 2023

deepakduggirala Oct 19, 2023

deepakduggirala Oct 19, 2023

deepakduggirala Oct 18, 2023

deepakduggirala Oct 18, 2023

deepakduggirala Oct 18, 2023

ri-pandey Oct 19, 2023

deepakduggirala commented Oct 18, 2023 •

edited

Loading

ri-pandey commented Oct 19, 2023

deepakduggirala Oct 19, 2023

deepakduggirala Oct 19, 2023

86 pagination #97

86 pagination #97

Conversation

ri-pandey commented Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepakduggirala commented Oct 18, 2023 • edited Loading

ri-pandey commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ri-pandey commented Oct 18, 2023 •

edited

Loading

deepakduggirala commented Oct 18, 2023 •

edited

Loading