adapt-security/adapt-authoring-assets-old

Module for handling Adapt asset data and files

adapt-authoring-assets-old's Issues

maxFileUploadSize can probably have a default value

Will probably need to discuss to decide on a sensible value for this.

Route definition can probably be refactored to use the defaults

i.e. abstractApiModule.js#L108-L132

We also need a way to be able to update the asset DB records.

Will obviously still need the asset specific endpoints:

• serve
• thumb

Refactor schema to use existing extension schemas

• authored
• tags

This'll remove the need for createdAt, createdBy and tags fields.

What should we do with Thumbnail for Audio and Zip files

Yesterday @taylortom mentioned that only a few assets are shown at frontend, then I've tested locally and it worked fine locally, I'm wondering if it caused because audio and zip file doesn't have a thumbnail.

What should we do with Thumbnails for Audio and Zip files?

How to implement Tags module?

Are there any examples of how we use tags module, like how to tag and remove tag...

Query route should be post

To allow querying via body data (this will come by default if we switch to the api code #8)

Composite asset upload

A user should be able to upload a zip file that contain multiple files. These assets would effectively work as a single item for the purposes of selection and build.

This is not part of MVP requirements and probably requires a more detailed proposal.

adapt-security/adapt-authoring#70

Schema integration

schemas are not yet integrated properly

Look into making file serving a bit more uniform/dev friendly

The main thing I'm thinking here is to maybe merge the serve and thumb routes into one, and specify the thumb/size as a query string?

Think this should be discussed on the call.

api/assets/serve/1234567890?thumb=true

Security vulnerabilities need fixing

https://github.com/adapt-security/adapt-authoring-assets/security

Look into permanent deletion

Would need to be added in conjunction with #9.

(would remove need for _isDeleted).

Create poster image on video upload (assets module)

It would be nice to generate the poster image (video placeholder) just on video upload. Initially it's no big deal and can be done together with thumbnail generation. But some work remains to be done when it comes to the component implementation and we may have to offer an option to let the user replace it later on by a custom version.

adapt-security/adapt-authoring#211

Fix typo in assetsModule.thumbnailMaxHeigt

Should be thumbnailMaxHeight

Files shouldn't be uploaded to root folder

This may be an issue with my config settings, but I'm getting files uploaded to the root authoring tool folder. This should be configurable (probably with a default value). Probably shouldn't go in temp, as the expectation here is that temp can in theory be deleted at any point.

Error messages need to be moved to a lang json

Router route needs to be plural

i.e. asset => assets

Various linter errors

Have just properly set up the linter on my machine, and noticed there are various issues.
Nothing major, but might be worth discussing the approach for this at the next meeting.

Default values should come from schema

So no need to hard-code any defaults in the js (e.g. importaudios.js#L5-L16)

Should we remove the temp files from zip?

During unzip process, the module will create a temp folder to saving the unzipped files, should we delete this temp folder with temp files after the process is finished?

Additional CRUD API routes

This issue if for tasks in addition to the basic asset upload/download.

• Able to permanently delete an asset (must)
• Able to replace an asset file (should)
• Able to update the title, description and tags for an asset (must)
• Able to upload a zip file containing multiple assets (should)
• Able to tag all assets and add a title and description when uploading and asset file or multiple files in a zip (should)

adapt-security/adapt-authoring#68

Code needs documenting

Failed to upload multi-files

If we upload multi-files, we will get empty objects.

Might be nice to expose acceptableTypes to config

Using the current value as default.

Revisit API endpoints

https://docs.google.com/document/d/19veTYxGN96B30pt80gaPam25hxoRkseaxnBpo5sAYLk/edit#

Existing

- `/` `GET`
- `/:id` `PUT`
- `/serve/:id` `GET`
- `/thumb/:id` `GET`
- `/trash/:id` `PUT`
- `/query` `POST`

Suggested?

- `/:id?` `GET`
- `/:id` `PUT`, `DELETE`
- `/serve/:id?thumb=true` `GET`
- `/query` `POST`

Typo in file path

File is assetsUtils.js

https://github.com/adapt-security/adapt-authoring-assets/blob/06a8419e9145fe45e38c2f45cfc62861c13001a3/lib/assetsrepository.js#L4

Need to set up route permissions

Temp formidable file location should be configurable

If we switch to using the adapt-authoring-middleware, it can be done via that (#11).

Remove formidable dependency

File uploads should instead go via MiddlewareModule#fileUploadParser. See AdaptFrameworkUtils#handleImportFile for a usage example.

Shouldn't need to specify config defaults in js

i.e. assetsmodule.js#L14-L16

The config module should make sure any defaults in the config schema are populated already.

Error on uploading single asset

I'm getting an issue when uploading a single asset to [POST] {{server_host}}:{{server_port}}/api/assets

If I try and post a single asset (.jpeg) I get the following error:

error server status code must be a number

Could be related to this line.

Code style changes

Some of these will be picked up by the linter (https://github.com/adapt-security/adapt-authoring-assets/issues/6). But some general points:

• Early returns (i.e. checking for an error as soon as possible and exiting the function)
• async/await over callbacks
• Nicer to throw Error instances (these are also expected in most automatic error handling (i.e. if passing to the next callback))