[WIP] Support exploring archive files attached to reviews.
Review Request #11402 — Created Jan. 24, 2021 and updated
Information | |
---|---|
mfliu | |
Review Board | |
master | |
Reviewers | |
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 | |
---|---|
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 ... |
|
|
F401 'tarfile.TarFile' imported but unused |
![]() |
|
Should be using single quotes |
|
|
These should all be single quotes as well |
|
|
single quotes in the print statement for strings |
|
|
Let's make sure that everything after base here stays in alphabetical order. |
|
|
This should inherit from object |
|
|
This should inherit from object |
|
|
I'm sure it's already on your radar, but this is kind of backwards. We should check explicitly for application/x-tar when ... |
|
|
We can probably leave this out for now. Unless you're planning on using backbone routing to add a bunch of ... |
|
-
-
reviewboard/static/rb/js/models/archiveReviewableModel.es6.js (Diff revision 1) 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.
Change Summary:
Update the Zip archive parsing to use intermediary class hierarchy instead of directly parsing to a Python dictionary
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+488) |
Checks run (1 failed, 1 succeeded)
flake8
-
-
-
-
reviewboard/reviews/ui/archive.py (Diff revision 2) single quotes in the print statement for strings
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+570) |
Checks run (2 succeeded)
Change Summary:
Add tar file support and switch to using Underscore template for very basic rendering of hierarchy
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+694) |
Checks run (2 succeeded)
-
-
reviewboard/reviews/ui/__init__.py (Diff revision 4) Let's make sure that everything after
base
here stays in alphabetical order. -
-
-
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 theTarArchiveFormat
, and theelse
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
-
reviewboard/static/rb/js/views/archiveReviewableView.es6.js (Diff revision 4) 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.
Change Summary:
Rebase to tip of master
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+694) |
Checks run (2 succeeded)
Change Summary:
Address review comments
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+670) |