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.

Sign up for GitHub

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

Jump to bottom

Handle Zipped Document Bundles Properly — Funded #117

Closed
freelawbot opened this issue Apr 16, 2015 · 9 comments
Closed

Handle Zipped Document Bundles Properly — Funded #117

freelawbot opened this issue Apr 16, 2015 · 9 comments
Labels
enhancement New Client

Comments

@freelawbot
Copy link

Issue by thinkcomp
Wednesday May 08, 2013 at 07:55 GMT
Originally opened as https://github.com/freelawproject/recap-firefox/issues/10

I just downloaded a bunch of documents for a case I filed (CAND). The PDFs got uploaded through RECAP without a problem, but the .zip bundles didn't make it. Specifically, for...

http://ia601700.us.archive.org/20/items/gov.uscourts.cand.265908

...I downloaded documents (2) containing 2-main.pdf and 2-1.pdf, (6) containing 6-main.pdf and 6-1.pdf, and (8) containing 8-main.pdf and 8-1.pdf.
@freelawbot
Copy link
Author

Comment by mlissner
Tuesday Apr 07, 2015 at 21:41 GMT

I can say with pretty high certainty that we aren't going to support zips. There's a lot of complexity to them and I just don't expect to be able to sort it out. What we could do is something that discourages people from using zips. That might be an option, so I'll leave this open. If anybody has ideas for ways of doing something like that, I'm all ears.

@mlissner mlissner removed the wontfix label Apr 22, 2015
@mlissner
Copy link
Member

mlissner commented Jan 3, 2017

A simple solution here would be to inject some kind of warning about the zip files either before the user clicks the link or after they do (as a modal, say). Another user just got bit by this.

@mlissner mlissner changed the title Support for Zipped Document Bundles Add Warning about Zipped Document Bundles Jan 3, 2017
@mlissner
Copy link
Member

Depending on the format of the zip files, the new RECAP server could probably handle this. I suggest handling this by:

  1. Tweaking the extension to upload zips.

  2. Track how many of these we get vs regular uploads.

  3. Make a decision as to whether we want to bother with them or throw an error.

@johnhawkinson
Copy link
Collaborator

johnhawkinson commented Nov 14, 2017 •
edited
Loading

Track how many of these we get vs regular uploads.

This is kind of chasing our tail. "Everybody" knows that ZIP files don't work, so people who care about RECAP don't use them. There are cases where I would download ZIPs, but I don't. (Or cases where I am willing to forgo RECAP for reasons of urgency.)

The better question would be to try to bound how often a ZIP download would be useful to the user. For me, that would be the time at which I am downloading all attachments to a single document, with increasing utility with a greater number of attachments. Zip isn't very important when there are 1 or 2 attachments, but it's super-helpful when there are 30.

For instance, http://ia600805.us.archive.org/19/items/gov.uscourts.mad.191926/gov.uscourts.mad.191926.docket.html has 66 attachments to doc 1. (I can't give you the CL docket now because the CL search seems to be down.)

@mlissner
Copy link
Member

mlissner commented Nov 14, 2017 •
edited
Loading

The good news is that the zip file is blissfully simple:

screenshot from 2017-11-13 20-55-12

Really, we could probably just start collecting these. The processing, unless I'm missing something, is a piece of cake:

  • Open the zip
  • Match up file names to document and attachment numbers
  • The rest of the merging and saving routines we already have.

Really it's just parsing file names (easy) and opening a zip (easy, but I forget how). This is about as easy as it comes. Forget tracking.

@mlissner
Copy link
Member

Just got another email from a user bit by this.

@mlissner mlissner changed the title Add Warning about Zipped Document Bundles Handle Zipped Document Bundles Properly Feb 8, 2018
@mlissner
Copy link
Member

mlissner commented Feb 8, 2018

Happy to share this feature got sponsored by a user that was annoyed by it. Looks like we'll be building it out! First steps:

  • Tweak the extension to send the zips & release new version
  • Tweak the RECAP API to receive the zips and store them somewhere

After that:

  • Get the parser going for the zips.

mlissner added a commit to freelawproject/recap-chrome that referenced this issue Feb 15, 2018
@mlissner
refactor(recap): Moves one function to use a listener as an example
81462e1 
BACKGROUND
~~~~~~~~~~
This is a step in the direction of nuking importInstance and
exportInstance, and towards doing things the "normal" way, via message
passing and listening.

As mentioned in freelawproject/recap#237, the way we're doing this now
makes it difficult to use certain functions, because those functions
can only be used in background scripts that are not *also* content
scripts. Since I need one of those functions to support zip upload in
freelawproject/recap#117, I'm working to get this fixed.

The only alternative to this that I could think of was to create a new
file called recap_background.js, which would be a background-only script
and then put my code in there, but I didn't love that approach since it
separated things arbitrarily.

IMPLEMENTATION
~~~~~~~~~~~~~~
The way I've implemented this is to move the uploadDocument function
out of Recap(), and to put it under a listener at the bottom of the
file. This listener checks for a package name of "recap", and then uses
an if statement to listen for a function mame of "uploadDocument". The
idea here is that I will move *all* of the existing functions in the
Recap() object into this listener, and it will use the package check and
function name check to run the right code.

I've done a bit of research on this approach, and it appears that
there's no right way to do this. This approach feels fairly clean to me
though, so I think I'm comfortable going with it. Other approaches
I considered were:

 - Using one listener per function. I like how this would allow us to
   have a cleaner separation of the functions (they're not just
   separated by if statements, but I didn't like how this would mean a
   lot of repetition of the package/function filtering within the
   beginning of each listener.

 - Using a simpler approach to function/package filtering. Could the
   function name just be "recap.uploadDocument" for example instead of
   using to keys in the object that we pass to the listener?

 - A few other things that aren't coming to mind right now (it's late).

BIG QUESTIONS
~~~~~~~~~~~~~
1. Can I refactor all of the functions to this format without breaking
   things? This is a big refactor, and thus dangerous. Going to need
   test coverage.

2. Is there a way we can simplify sendMessage? We went from 4 lines and
   one function call to 12 lines and one function call. That's not
   great.

3. What about tests? How do we pull this apart so that it's easy to
   also refactor the tests. This seems painful.
@mlissner mlissner mentioned this issue Feb 15, 2018
Closed
@mlissner
Copy link
Member

mlissner commented Feb 28, 2018 •
edited
Loading

Another todo:

  • Add the sponsor's name to our sponsorship list.

@mlissner mlissner mentioned this issue Aug 6, 2018
Open
@mlissner mlissner changed the title Handle Zipped Document Bundles Properly Handle Zipped Document Bundles Properly — Funded Jul 10, 2019
@mlissner mlissner mentioned this issue Feb 6, 2020
Closed
@mlissner
Copy link
Member

Fixed in freelawproject/recap-chrome#113

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New Client
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mlissner @brianwc @johnhawkinson @freelawbot