add AssetTemplate #400
add AssetTemplate #400
Conversation
paulapreuss
left a comment
paulapreuss
left a comment
There was a problem hiding this comment.
Thank you for this! 🚀
I think it will be a cool feature once it is done. I agree about the need to have some sort of management over the created template, I created a dedicated issue here #403. We can already use the changes in this PR to start testing on the staging server and collect feedback about how the feature should be further developed / how the handling should be for the things that are unclear. Once the functionality is clear, we can start to implement the design suggestions from #283.
There was a problem hiding this comment.
Is there a reason you are adding the template fields in the raw html instead of implementing it through a ModelForm from AssetTemplate? That would probably be easier to style given our current formats and there is less maintenance to be done if we end up changing any of the fields.
There was a problem hiding this comment.
I wanted the template code to be explicit and readable and did not intend to use a Django form to send template data back when saving. If it were part of AssetCreateForm, the fields would be rendered alongside the other fields, so I would have needed some more custom filters. If it were not (making this its own form), it's even more complicated, since the HTML is replaced and part of another form. And I would have needed to send that other form data for each request.
I found it easier to just build the necessary fields myself and then simply posting data back via fetch. Building new FormData is required because we need to differentiate between asset data and template data (like template name and description). I am reluctant to send both in the same payload and splitting information server-side. Easier to just dump the asset data in the frontend and appending that as a neat little package to our post, so the backend does not need to actually do inspection and can just save to DB. Also, building FormData from scratch is nothing new, the same mechanic is used when saving assets.
Styling should actually be easier because we have direct access to all fields, without Django magic in-between.
| if (typeof value === "string") | ||
| data[key] = value; | ||
| }); | ||
| const templateFormData = new FormData(); |
There was a problem hiding this comment.
Like here, for example, it seems like it would be cleaner to already have the form data instead of creating a new object here and populating it.
- make templates available for storage assets - show templates even if saving asset failed
stefansc1
requested review from
Bachibouzouk
and removed request for
paulapreuss
implements #283
Add new "AssetTemplate" model with page in Django admin.
Add "template" section to asset modal, collapsed by default.
In this section, a user can save a template for a) just this project or b) their account. In addition, they may request to make it available to all users. Users can give their templates a name and an optional description. For admin purposes, asset type, creation date, associated project and user are also saved. Asset data is saved as JSON in DB.
A user may also load an existing template (if one exists with a matching asset type and visibility), filling out all asset form fields. The asset form must still be saved manually (in case the user wants to make any changes, which has no effect on the template).
Differences to proposal, general observations: