Create web UI for file storage mgmt

dstpierre commented 3 years ago

A simple web UI to upload, view and delete files.

Uploading and deleting should trigger a message since if the user was using that file somewhere they might want to update the document(s) where it's used.

VladPetriv commented 2 years ago

Hey, I want to work on this issue

dstpierre commented 2 years ago

Hey @VladPetriv this is great!

Those are the available functions for files at the moment:

In internal/persister.go - the interface for all data access functions.

// Files / storage
AddFile(dbName string, f File) (id string, err error)
GetFileByID(dbName, fileID string) (f File, err error)
DeleteFile(dbName, fileID string) error

Those are the tasks I think are required for this (in order I'm thinking of them).

New function to list all files for an account

You'll need to add one function in persister.go for the files / storage related functions. Something like ListFiles or ListAllFiles that accepts an optional AccountID and return a []File, error for instance:

ListAllFiles(dbName, accountID string) ([]File, error)

This is just a suggestion. You might prefer to pass the optional accountID into a filtering parameter maybe for instance:

ListAllFiles(dbName string, filter ListFileFilter) ([]File, error)

Where ListFileFilter is defined in the internal/storer.go and has other filtering option you'd want to implement from the UI. For example, it could have thie AccountID but also potentially a way to add start and end date or anything you think is valuable for the owner to find uploaded files.

One you decide about that, you may implement this function in all three database provider:

database/postgresql in storage.go
database/mongo in storage.go
database/memory in storage.go

I'd suggest writing the test once in one of those provider, and copy pasting your test(s) functions in the other provider. See database/postgresql/storage_test.go for a starting point. You could create another test that will test your ListAllFiles function.

UI and its web handler

From there you'll have all you need to start the UI or the handler(s).

All UI handlers are in ui.go you may add new routes in the server.go line: 228.

The template are in templates directory.

Some aspects that would be nice to have in the UI (just suggestion here, feel free to skip or add new idea you have).

A way to filter by AccountID. The end user of the UI is the "owner" of the "App" so they might want to filter for uploaded files for one of their user, and the way to list that is via an AccountID.
This should handle paging. There's already a structure used the the ListDocuments and QueryDocument that contains the needed fields for paging. See ListParams in the internal package.

View and delete file

In my original description I was referring to the possibility to "upload" in this page. I'm not even sure it's required. What do you think? Since the user of this UI is the "App" owner, why they'd like to upload file on behalf of their user. Maybe there's a use case for that, I'm not totally sure.

To view the file, you'll have the URL field populated from the new ListAllFiles function.

Idea: Can we display files like thumbnail ala Google Drive with image preview or icon for known types. Depending on how far you'd like to go. I'll let the UI decision in your hand.

For deletion, you may call the Persister's DeleteFile function.

Triggering an event on deletion

I'll get back to you on that one, this will be a call in your UI handlers for the deletion. But I don't have all the info at hand at the moment.

Do not hesitate to ask any questions, start a PR and I'll happy to help you along the way with anything.

VladPetriv commented 2 years ago

In my original description I was referring to the possibility to "upload" in this page. I'm not even sure it's required. What do you think? Since the user of this UI is the "App" owner, why they'd like to upload file on behalf of their user. Maybe there's a use case for that, I'm not totally sure.

I guess that we need to add this feature because in my experience while using Firebase I have some cases when need to upload a file manually

VladPetriv commented 2 years ago

How do you think is it a good idea to add methods in the "internal.File" structure for getting a better date, type and size format and then call it in templates?

For "date" it will be something like it:

"2022-08-10 11:43:23.188463 +0000 +0000" -> "August 10, 2022 at 11:43"

For "size" it will be something like it:

"123456" -> "123.5 KB"

For "type" it will be something like it:

"abeSsF75dRVs/d74ab220-c565-47d5-b01b-84a96cb7a5b0/a9Z7jh7EfHkqXDrwBpF4NpmmNPgL32iu.jpg" - > "jpg"

VladPetriv commented 2 years ago

To view the file, you'll have the URL field populated from the new ListAllFiles function.

When I try to get file by his URL field. I was redirected to login page

dstpierre commented 2 years ago

@VladPetriv

I guess that we need to add this feature because in my experience while using Firebase I have some cases when need to upload a file manually

Fair enough, were you uploading file on behalf of a user or just files that needed to be uploaded? There's a way to get a token via an AccountID when being "root" which is the user of that UI interface. Let's keep this discussion open if upload need to be on behalf of a user or not.

helper function in internal.File

I think they'd be better in render.go in the customFuncs map definition for the UI functions.

I'd prefer keeping the API as raw as possible and since only the UI will use them for now, UI helper functions seems to be preferable.

When I try to get file by his URL field. I was redirected to login page

What are you using as backend? The CLI or the core package? There might be a bug in the local file package where the URL return is not correct. I'll double check this.

dstpierre commented 2 years ago

@VladPetriv I've fixed the issue for local storage serving. In fact that was the CLI that was doing this before, and now that the CLI uses the same core package I needed to add a way to serve file that are created with the local file storage.

You'll need to have the APP_ENV=dev environment variable for the server to serve the /tmp directory, which is where the local storage sstores it file.

VladPetriv commented 2 years ago

Fair enough, were you uploading file on behalf of a user or just files that needed to be uploaded?

I used the second variant. Maybe you're right and we really don't need to have this feature.

I'd prefer keeping the API as raw as possible and since only the UI will use them for now, UI helper functions seems to be preferable.

Yea it's look better then store method's into "File" structure.

VladPetriv commented 2 years ago

@VladPetriv I've fixed the issue for local storage serving. In fact that was the CLI that was doing this before, and now that the CLI uses the same core package I needed to add a way to serve file that are created with the local file storage.

You'll need to have the APP_ENV=dev environment variable for the server to serve the /tmp directory, which is where the local storage sstores it file.

Thanks) It start to work

VladPetriv commented 2 years ago

@dstpierre It will be good idea to store original name of file not only generated key. How do you think?

dstpierre commented 2 years ago

@VladPetriv hmm, this would involve changing the sb_files schema though on each "apps" / user databases.

It's up to the caller to set the file name. If you look at storage.go line:42 the filename passed to libraries while uploading (calling the API) is the last parameter of the file key.

You can grab this last path value and this should be the original file name passed when uploading.

dstpierre commented 2 years ago

That being said, I'm reviewing the code for the Go API client and the "name" form parameter is not filled, but the library accept a filename as parameter. I'll apply the fixes there and review the Node and JavaScript library to make sure they're using the name form param.

VladPetriv commented 2 years ago

storage.go Lines:42-45

name := r.Form.Get("name")
if len(name) == 0 {
   name = randStringRunes(32)
}

We can remove these lines of code and use filename from file which user uploaded.

In my opinion it's really more comfortable then fill one more field.

dstpierre commented 2 years ago

I've clean that part up, just pushed, if a custom name isn't provided the original filename will be used.

From that file key, the last part of that path will be the filename (original or forced by the API caller).

Hopefully that help

VladPetriv commented 2 years ago

That look much better than my variant. 😎

VladPetriv commented 2 years ago

Hey @dstpierre! Can you review my pr?

staticbackendhq / core