🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@berinhard
Copy link
Contributor

@berinhard berinhard commented Jan 12, 2022

This PR adds a new admin view that's is accessible through a new button at sponsorship detail page's header. The view is very straightforward and, for now, it only details the uploaded assets within a table.

New button on Sponsorship detail
Screenshot from 2022-01-12 19-34-21

Uploaded assets table
Screenshot from 2022-01-12 19-34-41

@ewdurbin I know we've spoken about the functionality to download all assets as a single zip file. I thought about adding a button at the bottom of the table with that option. The button should trigger a request with format=zip in the querystring and the backend will do the rest. But I thought about opening a new PR to address this exclusively. That way PSF staff would at least be able to see the data once we merge this PR.

Let me know what you think about this.

@berinhard berinhard requested a review from ewdurbin January 12, 2022 22:41
Django does not allow to select related nested FK from generic fks, such
as content_object. Because of that, a lot of collateral queries were being
executed to display the sponsorship name because it depends on both its package
and sponsor names. This commit changes this by filtering all the sponsorships
and organizing it as a in-memory dict that is only load for this page.
Since assets are created automatically by the process to handle new sponsorship
applications, having the possibility for users to manually add assets can lead
to states that are unknown to the application. This commit prevents that from
happening.
@berinhard
Copy link
Contributor Author

@ewdurbin this PR probably is now at a more stable state. It adds a admin view to list all the generic assets but a few smart filters to help PSF staff to work on top of them.

This is a print screen of the view listing all the assets from my local DB:

Screenshot from 2022-01-26 17-38-46

And this is a print screen of every filter being combined to reduce the number of listed assets:

Screenshot from 2022-01-26 17-38-34

I tried to leave the interface with hyperlinks to sponsor/sponsorship and also, if the asset has a file, to the file itself. The next step now is the admin action to export the assets as a ZIP file.

I though about using Django's admin actions as the user interface to enable the export. Thus, the UX would be something like:

1 - The user apply the filtering they want to use;
2 - They click on checkbox to select all the assets which are being listed;
3 - The user will select "Export assets as Zipfile" action;
4 - A confirmation page will be displayed;
5 - The ZIP will be downloaded if the user confirms it;

A minor disclaimer. There's no action select in the attached screen shots because the delete operation is disabled for the assets. So, since there's no action to be performed, Django admin hides the select and the entries' checkbox by default.

Anyway, @ewdurbin do you feel the process I describe above satisfies the needs to export the data? The only concern I have is with the memory consumption. I mean, if we process the whole zip within the lifecycle of the request, maybe we can run into issues if there are too many assets to be exported. An alternative to that would be to leave the export process to be async, using django-rq for example, and then send the zip file via email. But I don't know if this is big design upfront. How do you feel about this topic?

@berinhard
Copy link
Contributor Author

@ewdurbin another question about the ZIP file is: how to organize the files? For example, let's imagine that there are 10 assets to be exported, 5 texts and 5 images, being a pair of each from a different sponsorship. Should the zip be organized using the sponsorships ids as directories? So, something like:

- export.zip
|  - sponsorship_1/
|  |  - info.txt  (this could contain the sponsorship name and maybe a URL link to the admin edit page?)
|  |  - text_assets.txt
|  |  - img_asset.png
|  - sponsorship_2/
|  |  - info.txt
|  |  - text_assets.txt
|  |  - img_asset.png

And so on...

Or just using the sponsorship as the file prefix would be enough? Something like:

- export.zip
|  - sponsorship_1_text_assets.txt
|  - sponsorship_1_img_asset.png
|  - sponsorship_2_text_assets.txt
|  - sponsorship_2_img_asset.png

And so on...

Although the first strategy is more complicated, I guess it's more powerful and easier for the PSF staff to use. It also gives us the benefits to add the "info.txt" file to contextualize the application within the ZIP file by listing their basic information.

@ewdurbin
Copy link
Member

The process you note in the first question to select assets and use an action is 👍🏼

For the export... we probably want to use a temporary file/dir to assemble then zip files. Let's start doing it in the request cycle and see how it performs.

For structure, a folder per "Sponsor Name" is probably best!

@berinhard
Copy link
Contributor Author

berinhard commented Jan 27, 2022

@ewdurbin I pushed new commits implementing the export functionality. The sponsor name is used as the directory name, while the asset's internal_name is used as the filename.

Although the PR is missing tests for this new admin action (I'm planning to work on them tomorrow), I've generated a sample zip using my local database so you can review it. The sample zip contains examples of one text asset and one image assets, both from the same sponsor. Disclaimer: the data is very naive and are both placeholders I've input while testing the assets form.

Feel free to merge this if you want to or if this feature is being missed by PSF staff. I can keep a personal note as the missing tests as a technical debt that I can address on an upcoming PR.

@berinhard berinhard force-pushed the feature/view-uploaded-assets branch from c0177ae to 7e48eb4 Compare January 28, 2022 21:20
@berinhard
Copy link
Contributor Author

@ewdurbin the tests are here. Now this branch can be reviewed and merged with more confidence =)

@ewdurbin
Copy link
Member

ewdurbin commented Feb 7, 2022

Thanks @berinhard, I also added a small fix for the AssetsInline (which wasn't properly rendering status/values) and registered admin views for the new asset types.

@ewdurbin ewdurbin merged commit 6f0c8c8 into main Feb 7, 2022
@ewdurbin ewdurbin deleted the feature/view-uploaded-assets branch February 7, 2022 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants