fix: upload list pagination headers #1739

alanshaw · 2022-08-15T15:52:16Z

Fixes Link headers to include required pagination variables.
Adds "first" and "last" URLs to link header so pagination can operate properly.
Catches DB range error and re-throws it with the appropriate HTTP status 416.
Adds indexes on the uploads table for list items.
Fixes website to allow pagination above 1,000. Only loads the current page instead of everything.
Removes the search box - it only filters the current page so is almost useless. Needs proper integration with server side API.

Visual preview:

Note new "Request ID" column for pinned items:

resolves #1610
resolves #1501
resolves #1362
resolves #1737

github-actions · 2022-08-16T10:20:35Z

Website preview 🔗✨

build log

joshJarr · 2022-08-16T10:56:31Z

Nice! Code looks good and the BE changes are ace, I found a few issues with the FE code:

I can not unselect uploads if I navigate to a new page, and upon navigating back the upload does not appear selected, but If I click delete, it's removed.
Steps to replicate:
- On the uploads table the delete button is disabled until something is selected
- Select a (or many) uploads (checkbox), the delete button becomes enabled
- Navigate to the next page, the delete button remains enabled
- Navigate back to the previous page, the delete button is enabled but the upload is not visually selected.
Expected:
- Either selection is wiped upon navigating to a new page, or the selection is persisted when navigating back
- If selection is wiped, the delete button should become disabled.
Actual
- Selection is not visually persisted on the table
- Delete button is active, and clicking will delete the previously selected item.
(can't upload MP4, give me a shout if you want a higher res or a quick demo!)

We could just wipe the selection upon navigating away.

Total amounts of uploads/pins are incorrect on the tabs, only the selected result length is shown.
A quick win here could be as simple as removing the total number?

Pins and Uploads are broken and appear on the same table with table columns also breaking.
- Hopping between the two tables I find they sometimes stack together resulting in the screenshot below

There's a few more minor issues here:
- Sorting is also persisted through the upload & pin tabs.
- When I hit the http://localhost:4000/account/?table=pinned URL I am directed to the uploads table no pin, despite having pings

I think the table and pagination components are tightly coupled and need a bit of refactoring in order to separate pins and uploads, but reuse the same components. @flea89 has worked on decoupling them from the table here: #1700 and in https://github.com/web3-storage/web3.storage/pull/1699/files

I think the FE code here may require a bit more work, but the BE aspect is perfect! Would it make sense to split this into two PRs so that the Link URL, error handling changes & indexes aren't blocked by the UI?

Thanks!

joshJarr

Couple of questions on the API/DB side, but besides these all look good.
A few FE issues, is the plan to split these out into a separate ticket or to tackle 'em all here?

joshJarr · 2022-08-16T14:55:58Z

packages/website/components/account/fileUploader/fileUploader.js

@@ -109,7 +109,6 @@ const FileUploader = ({ className = '', content, uploadModalState, background })
            }}
            icon={<FolderIcon />}
            dragAreaText={content.drop_prompt}
-            maxFiles={3}


Has this been dropped for testing purposes? Or is it cool to no longer restrict file uploads?

There's no error feedback in the UI and nothing that says only 3 files at a time. I don't know why we'd restrict files here...

Yep, that's a great point - was just double checking

joshJarr · 2022-08-16T15:01:09Z

packages/website/components/account/filesManager/filesManager.js

-      getUploads();
-    }
-  }, [fetchDate, getUploads, isFetchingUploads]);
+    getUploads({ size: itemsPerPage, page, sortBy, sortOrder });


Might be good to reset the selectedFiles here too, so that hidden selections aren't stored in that state.

joshJarr · 2022-08-16T15:20:19Z

packages/db/index.js

-      query = query.gte('inserted_at', opts.after)
+  async listUploads (userId, pageRequest) {
+    let query
+    if (pageRequest.type === DATE_TIME_PAGE_REQUEST) {


I find the Date time pagination a little tricky,
Say I had two or more records with the exact same date time, wouldn't this break pagination & it would miss some results?
Super edge-casey, but am keen to understand why filtering by datetime is necessary, is it purely for the existing UI?

It's required by the pinning service API https://ipfs.github.io/pinning-services-api-spec/#section/Pagination-and-filtering and that's what was originally implemented.

Ahh ok, in which case should this also be replicated in listPsaPinRequests?

listPsaPinRequests serves the PSA (GET /pins) primarily. It's now date ordered explicitly (and you can't change the sort field or direction from the UI). So, I've left it mostly as is and dealt with the "page number pagination" at the API handler level instead. If we want to enable that UX for the PSA table then yes we can probably refactor to use this pattern also.

…upload-list-pagination-headers

francois-potato · 2022-08-25T11:31:12Z

packages/api/src/user.js

+  try {
+    data = await env.db.listUploads(request.auth.user._id, pageRequest)
+  } catch (err) {
+    if (err.code === 'RANGE_NOT_SATISFIABLE_ERROR_DB') {


Maybe reuse RangeNotSatisfiableDBError.CODE here

francois-potato · 2022-08-25T11:31:46Z

packages/api/src/user.js

-    console.error(e)
-    throw new HTTPError('No pinning resources found for user', 404)
+  } catch (err) {
+    if (err.code === 'RANGE_NOT_SATISFIABLE_ERROR_DB') {


Maybe reuse RangeNotSatisfiableDBError.CODE here

Great point! I could make a constant in the API package so that there's less dependancy on the db package, or import it over from the db package. am struggling to weight up which would be best!

francois-potato · 2022-08-25T11:39:56Z

packages/website/lib/api.js

+ */
+export async function deletePinRequest(requestid) {
+  const tokens = await getTokens();
+  if (!tokens[0]) {


A bit confusing; could we use tokens.length > 0?

Possibly, but with this we're checking if tokens[0] is truthy, as it could be null.
Tokens could be [null, null] and .length would be 2

…upload-list-pagination-headers

joshJarr · 2022-08-25T17:29:38Z

LGTM now conflicts are resolved, lots of changes here so would be great to get a second approval, @orvn @olizilla @vasco-santos are either you guys available to take a look?
@e-schneid would be good if you could take a quick look too, just to ensure I've not caused havoc merging the upload table changes.
Thanks!

…upload-list-pagination-headers

joshJarr · 2022-08-30T16:19:58Z

Merging this since it resolves a fair few issues that are currently live, but if anyone else has any feedback feel free to leave it and will address in a separate PR.
Thanks!

alanshaw · 2022-09-05T09:07:14Z

@joshJarr thanks for finishing this!

Did the indexes get applied in prod? I just realised I should have written a migration for applying them.

ref https://github.com/web3-storage/web3.storage/pull/1739/files#diff-c387c1cfe9b20b289a2e4b668a020d576f0c3acda4c9379bef0a7c04718ecc11

fix: upload list pagination headers

bff2745

alanshaw requested review from olizilla, orvn, vasco-santos and joshJarr August 15, 2022 15:52

alanshaw marked this pull request as ready for review August 15, 2022 15:52

alanshaw added 3 commits August 15, 2022 17:36

fix: test

708906a

fix: separate components

acc6221

Merge branch 'main' into fix/upload-list-pagination-headers

113e508

fix: use specific type of pagination

a34764c

joshJarr suggested changes Aug 16, 2022

View reviewed changes

alanshaw and others added 9 commits August 17, 2022 16:04

fix: all the things

bb726be

fix: db tests

4142562

fix: api tests

861614c

fix: cron tests

b82e838

fix: website tests

43bfefa

chore: appease website linter

ae613aa

Merge branch 'main' of github.com:web3-storage/web3.storage into fix/…

ecc3fe9

…upload-list-pagination-headers

Merge branch 'main' of github.com:web3-storage/web3.storage into fix/…

45c8ca0

…upload-list-pagination-headers

chore: fix missed merge conflict.

1233d52

francois-potato mentioned this pull request Aug 25, 2022

New table component: reusable, accessible and api pagination compliant #1711

Closed

5 tasks

francois-potato reviewed Aug 25, 2022

View reviewed changes

joshJarr added 3 commits August 25, 2022 17:06

Merge branch 'main' of github.com:web3-storage/web3.storage into fix/…

8b1bd04

…upload-list-pagination-headers

chore: fix ilinting

4984510

chore: rm unessesary failing test

f3088d9

joshJarr approved these changes Aug 25, 2022

View reviewed changes

joshJarr requested a review from e-schneid August 25, 2022 17:29

joshJarr added 3 commits August 30, 2022 16:47

Merge branch 'main' of github.com:web3-storage/web3.storage into fix/…

56fe4c6

…upload-list-pagination-headers

chore: fix linting on website.

bd02502

chore: more linting fixes!

973a922

joshJarr merged commit 2ffe6d7 into main Aug 30, 2022

joshJarr deleted the fix/upload-list-pagination-headers branch August 30, 2022 16:20

This was referenced Aug 30, 2022

chore(main): release website 2.17.0 #1771

Merged

chore(main): release cron 1.6.0 #1800

Merged

chore(main): release api 7.4.0 #1792

Merged

mbommerez mentioned this pull request Sep 2, 2022

Implement the new /user/pins end point in FE #1830

Closed

joshJarr mentioned this pull request Sep 5, 2022

FE - Upload & Pins file manager table refactor #1836

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: upload list pagination headers #1739

fix: upload list pagination headers #1739

alanshaw commented Aug 15, 2022 •

edited

Loading

github-actions bot commented Aug 16, 2022 •

edited

Loading

joshJarr commented Aug 16, 2022 •

edited

Loading

joshJarr left a comment

joshJarr Aug 16, 2022

alanshaw Aug 16, 2022

joshJarr Aug 16, 2022 •

edited

Loading

joshJarr Aug 16, 2022

joshJarr Aug 16, 2022

alanshaw Aug 16, 2022

joshJarr Aug 17, 2022

alanshaw Aug 17, 2022

francois-potato Aug 25, 2022

francois-potato Aug 25, 2022

joshJarr Aug 26, 2022

francois-potato Aug 25, 2022

joshJarr Aug 30, 2022

joshJarr commented Aug 25, 2022

joshJarr commented Aug 30, 2022

alanshaw commented Sep 5, 2022

fix: upload list pagination headers #1739

fix: upload list pagination headers #1739

Conversation

alanshaw commented Aug 15, 2022 • edited Loading

github-actions bot commented Aug 16, 2022 • edited Loading

Website preview 🔗✨

joshJarr commented Aug 16, 2022 • edited Loading

joshJarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshJarr Aug 16, 2022 • 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

joshJarr commented Aug 25, 2022

joshJarr commented Aug 30, 2022

alanshaw commented Sep 5, 2022

alanshaw commented Aug 15, 2022 •

edited

Loading

github-actions bot commented Aug 16, 2022 •

edited

Loading

joshJarr commented Aug 16, 2022 •

edited

Loading

joshJarr Aug 16, 2022 •

edited

Loading