[WIP] Support exploring archive files attached to reviews.

Review Request #11402 — Created Jan. 24, 2021 and updated

mfliu
Review Board
master
reviewboard, students
ReviewBoard currently supports attaching individual files for review,
but while some filetypes can be opened in-browser and examined, archive
files can only be downloaded. This commit adds a new review UI that
allows archives to be browsed in ReviewBoard without downloading the
file. Users can navigate the archived files, see metadata, and download
individual files.


Summary
[WIP] Support exploring archive files attached to reviews.
Description From Last Updated

This model will have all of the data that the view will display. The templates will json-encode the return value ...

daviddavid

F401 'tarfile.TarFile' imported but unused

reviewbotreviewbot

Should be using single quotes

ryankangryankang

These should all be single quotes as well

ryankangryankang

single quotes in the print statement for strings

ryankangryankang

Let's make sure that everything after base here stays in alphabetical order.

daviddavid

This should inherit from object

daviddavid

This should inherit from object

daviddavid

I'm sure it's already on your radar, but this is kind of backwards. We should check explicitly for application/x-tar when ...

daviddavid

We can probably leave this out for now. Unless you're planning on using backbone routing to add a bunch of ...

daviddavid
david
  1. 
      
  2. This model will have all of the data that the view will display. The templates will json-encode the return value from your ArchiveReviewUI.get_js_model_data() method and pass that into the constructor for your model here (which is how data gets from the Python on the server into JS on the client). In this case you'd probably have some kind of tree data representing the archive contents.

    The view then takes the data stored in the model and displays it.

  3. 
      
mfliu
Review request changed

Change Summary:

Update the Zip archive parsing to use intermediary class hierarchy instead of directly parsing to a Python dictionary

Commits:

Summary
-
[WIP] Support exploring archive files attached to reviews.
+
[WIP] Support exploring archive files attached to reviews.

Diff:

Revision 2 (+488)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
  1. 
      
  2. reviewboard/reviews/ui/archive.py (Diff revision 2)
     
     

    Should be using single quotes

  3. reviewboard/reviews/ui/archive.py (Diff revision 2)
     
     

    These should all be single quotes as well

  4. reviewboard/reviews/ui/archive.py (Diff revision 2)
     
     

    single quotes in the print statement for strings

    1. These are just for debugging, and will be removed in the actual review-request

  5. 
      
mfliu
mfliu
david
  1. 
      
  2. reviewboard/reviews/ui/__init__.py (Diff revision 4)
     
     
     
     
     

    Let's make sure that everything after base here stays in alphabetical order.

  3. reviewboard/reviews/ui/archive.py (Diff revision 4)
     
     

    This should inherit from object

  4. reviewboard/reviews/ui/archive.py (Diff revision 4)
     
     

    This should inherit from object

  5. reviewboard/reviews/ui/archive.py (Diff revision 4)
     
     
     
     
     
     
     

    I'm sure it's already on your radar, but this is kind of backwards. We should check explicitly for application/x-tar when instantiating the TarArchiveFormat, and the else clause should raise.

    Inn fact, we might want to have a dict mapping mimetype to the format class so it can just be something like:

    try:
        archiveFormat = self.formats[self.obj.mimetype]
        archive = archiveFormat(self.obj.filename, file_bytes)
        data['archive_hierarchy'] = archive.to_dict()
    except KeyError:
        raise NotImplementedError
    
    1. I added the formats dict mappings, which works quite well. I did want to check what the expected behaviour for unsupported archive formats is. Should they not be handled by the review UI at all (in which case I just omit the mimetypes from the dict), or should they be handled but error out somehow?

    2. I think we still want to catch the KeyError (just for bullet-proofing), but we should also only put things into supported_mimetypes if the review UI can handle them.

  6. We can probably leave this out for now. Unless you're planning on using backbone routing to add a bunch of separate URLs that are all served by the same view, we won't need this.

  7. 
      
mfliu
mfliu
Review request changed

Change Summary:

Address review comments

Commits:

Summary
-
[WIP] Support exploring archive files attached to reviews.
+
[WIP] Support exploring archive files attached to reviews.

Diff:

Revision 6 (+670)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...