harvard-lil / perma

Indelible links
417 stars 71 forks source link

Add multiple captures feature #3521

Closed tinykite closed 4 months ago

tinykite commented 4 months ago

Summary

Rebuilds the multiple link batch capture feature within Vue.

Key updates

UI Changes

  1. As discussed in Slack, I made a small change to the select element included for folder selection:

    • Previously the select would always display all top-level folders. And if a user selected a nested folder from the folder tree UI, it would display that selected option plus all child sub-folders of the affiliated top-level folder.
    • What has now changed is that the select will still always display all top-level folders. If a user selects a nester folder from the folder tree UI, however, only that additional option will populate within the folder select — which matches the behavior of the custom select. We plan to revisit this in the future during a redesign.
  2. There were two bugs I noticed in the legacy batch capture UI:

    • If a batch capture included no successful captures, it would display that the batch progress was "NaN" complete.
    • For every completed url batch I tested, the total batch progress was never calculated higher than 67%.
    • An easy fix for both issues that didn't require much of a refactor is to render "batch complete" instead of "batch [percent] complete" when all links in a batch have either captured successfully or failed.

Screenshots

The batch capture trigger ("create multiple links") looks like a link, but is semantically a button. It will change styling based on if the selected folder is private: Screenshot 2024-05-21 at 9 44 51 AM

The new dialog element minimally wraps the previous modal styling: Screenshot 2024-05-21 at 9 45 04 AM

For now, if a capture fails the error will be counted — and communicated inline — but the specific error (for example, an invalid URL) is not intended to be rendered just yet. This will happen in follow-up work: Screenshot 2024-05-21 at 9 45 20 AM

If a capture succeeds, the dialog will display a link to view the completed archive. Additionally, if all captures are complete, an option to download a CSV describing the capture data for all captures is provided: Screenshot 2024-05-21 at 9 45 41 AM

After a batch capture is initiated, the new Vue app will dispatch to the existing batch capture list area of the application, which should update with a new entry for each batch capture. Clicking the batch capture link will display batch capture details in the legacy modal:

Screenshot 2024-05-21 at 11 42 28 AM Screenshot 2024-05-21 at 11 45 06 AM

To Do

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.60%. Comparing base (5f73df1) to head (f52484c). Report is 1 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3521 +/- ## ======================================== Coverage 68.60% 68.60% ======================================== Files 48 48 Lines 6795 6795 ======================================== Hits 4662 4662 Misses 2133 2133 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tinykite commented 4 months ago

What would you think of doing that here as well, E.g. Test Library > Subfolder > Selected Subfolder instead of plain Selected Subfolder?

Sure thing. This is handled in 45c280d

Looks like the "batch complete" and folder message are rendering along with the spinner, right when you hit submit, before the API returns a response:

This is fixed in d0734ed. The batch details won't be visible until the batch is officially queued.

I saw this go by in the console, thought I'd share Ope, this type mismatch was an oversight that happened from me generally having the Spinner component entirely commented out. Fixed in fd54cf0

I noticed that you can submit this form without selecting a folder, and if you do, the modal just closes. I know you are planning to add error handling later, separately, but wanted to flag in case that isn't how you wanted this for the time being.

I penciled in placeholder error handling for this in d142069, so that it doesn't get lost or forgotten about while I'm working on error handling for both the single and batch capture. While this placeholder error handiling will still just close the dialog element without any UI indication for now for the batch captures, it will also actually render the correct error UI (along with a more-specific, pencilled-in message) for the single link capture.

Finally.....did I see what I saw????

I wasn't able to reproduce this issue across multiple test user accounts, however I did notice that if there was no selected folder set in cookies there was a bug where "Please select a folder" wouldn't be selected and an empty option would be added to the select. This has since been fixed (with 0978ec2 and 25fe0ac).

I will absolutely continue to try to test this, and noodle about if there's anything I can think of that would only happen in one specific use case. One thing I have been thinking about that could potentially make it easier to reproduce bugs is exploring UI development in Storybook. Right now, as I develop specific UI features I try to regularly switch between user accounts, and do my due diligence to test across as many use case as possible. But it's certainly not a perfect process — and there's a lot of room for error when testing for one user and then making a tiny fix while testing for another.

Storybook is a framework-agnostic tool intended to help with this — I don't have a proposal for its use just yet, but might be fun to experiment with during 20% time on a Friday.

One more thing: I noticed there's a couple tiny merge conflicts. I'll handle them early tomorrow morning and plan to double-check everything is working as expected before merging.