[WIP] Backbonize FileList

[PVince81]

No description provided.

[PVince81]

FileInfoModel is the base. From here one can either call fetch() to get both the model and the directory contents collection if it's a folder (PROPFIND returning both), else it's just a file (PROPFIND).

Goal is to try to move as much logic as possible to the models or collection like sorting and file actions.
Challenge is to decide whether to continue using OC.Files.Client (based on davclient) or use the Backbone Webdav sync instead, or a hybrid of both. Ideal would be to have only one.

CC @DeepDiver1975

[PVince81]

Backbone models should have an id. In theory the real ID is actually the Webdav HREF, not the file's file id (which we can store in a separate property). Path and name can be derived from the href.
Renaming or moving a file is basically changing its Webdav id even though the internal id stays the same.
I'll see if it makes sense to fit it this way into the JS code.

[PVince81]

Finally, that's some proper backbone here !

• The FileList is now more like a view and listens to collection changes.
  Adding/removing models to the collection will update the list visually: https://github.com/owncloud/core/pull/25909/files#diff-dafefd9519924bec4efa9f5af4af8e68R233
• The selection is now also a collection, which automatically deduplicates entries when re-added. Adding/removing models from there will update the UI to reflect whatever is selected through the collection events: https://github.com/owncloud/core/pull/25909/files#diff-dafefd9519924bec4efa9f5af4af8e68R290
• FileSummary doesn't need handholding any more, no more explicit update: updates of the collection (also selection collection!) will automatically update the summary calculation: https://github.com/owncloud/core/pull/25909/files#diff-57e2634bbe6a593fc10f0e05ac014184R50

[PVince81]

there's still a lot of work to be done, I'll likely move all the filesClient actions directly to the model. Goal is that anyone with access to the collection/model (sidebar, alternate view) can still perform actions on file/folders without any direct connection to the list

[PVince81]

Just made more progression, now the following operations are moved to the model

• fetch after insert (for upload and create new folder)
• rename
• move
• delete

The remaining ones:

• filesClient.createDirectory: might need a special method on the model model.createDirectory()
• reload: this must use model.fetch() on the current directory's model, trouble is with custom webdav properties
• filesClient.putFileContents: for creating a new file with string contents, can be moved to the model as well
• maybe add a "triggerDownload()" method on the model to trigger a download of that file

Once this is done the FileList doesn't need to know anything about FilesClient any more and can receive the model/collection injected from the outside !

Some challenges:

• custom webdav properties, how should plugins like favortiesplugin/tagplugins inject these ?

[PVince81]

• to avoid breaking apps that extend the old FileList, implement the backbonized file list, etc as a separate class. Then all other lists can slowly get ported to the new structure

[PVince81]

• idea: instead of calling them model/collection, use the "Node" terminology from core and possibly even the same methods. One difference though would be that the methods could return promises instead of the objects.

[PVince81]

hmm, I realize that we need the concept of "file action class" (command pattern) because we want file actions to be undoable but also resumable in a modified state. The resumable case is for example when moving/copying a file and the conflict dialog pops up.

[PVince81]

file action command pattern concept ideas: #26296

[PVince81]

@mjobst-necls since you already got to know a lot of the mess that is the FileList code, I'd appreciate if you could give me some feedback on this PR 😄

[PVince81]

To summarize quickly: the goal is to move the file action logic to the backbone models and collections. Then the views (FileList needs to become one and MainFileInfoView is one too) only listen to model/collection changes to adjust themselves accordingly.

[PVince81]

... and additionally the Views also gain the ability to execute file actions directly on the models like refetching properties, renaming, etc.

[PVince81]

Rebased. Make sure to rerun make when checking out this branch as it brings a new lib.

[mjobst-necls]

Well, guys. I'm currently working on a Diagram to get a better overview of the javascript code.
I can tell you - there is really a lot of work to do.

Here is a link, if someone is interested:
http://jsfiddle.net/yke5g47v/show/

The model is not correct at all and not complete, but it supports understanding pretty much, I think.

[mjobst-necls]

While adding the all the functions to the OCA.Files.FileList, I saw a lot of function that are marked as deprecated. How long do these function have to exist?

If they have to be existing for longer term, they could at least be moved to a separate file like 'filelist.deprecated.js' to make the filelist.js more clear.

What do you think?

[mjobst-necls]

The coupling of the main JS-objects is very high.

The collection attribute of OCA.Files.FileList is only dependent on the model. We should remove that attribute and get the collection out of the model.

The new button should be a separate object of type OCA.Files.NewFileButton. The following functions/attributes can be moved to this type:

• _renderNewButton()
• _onClickNewButton()
• _addButtonTemplate
• _newButton
• _newFileMenu : this attribute is only used by the newFileButton

The sort functionality should be a part of the OCA.Files.FileInfoCollection or even better capsulated in a new class like OCA.Files.FileSort. The following function/attributes could be moved:

• _sort
• _sortDirection

The class OCA.Files.FileList contains properties like breadcrumb, newbutton, _selectedCollection, _selectionSummary, _lastChecked, _dragOptions, _folderDropOptions, _uploader that are not used by OCA.Files.FavoriteFileList, OCA.Sharing.FileList, and OCA.Trashbin.FileList. We could create a new OCA.Files.FileListBase that only provides general behavior.

Because the OCA.Files.FileList is a view, it should not have a filesClient attribute, but be able to create a new instance. The filesClient is used in OCA.Files.FileList with its functions

• reload => filesClient.getFolderContents(): reload() should partially be a part of the model
• createFile() = > filesClient.putFileContents(): createFile() should partially be a part of the collection/model? (not sure at all, but a file is added to collection and not to a model, so I think it should be a part of the collection)
• createDirectory() => filesClient.createDirectory(): createDirectory() should be treated like createFile()
• getPropfindProperties: filesClient.getPropfindProperties() does not need an instance of OC.Files.Client so that it can be used like a static function.

Challenge is to decide whether to continue using OC.Files.Client (based on davclient) or use the Backbone Webdav sync instead, or a hybrid of both. Ideal would be to have only one.

I propose that we keep the OC.Files.Client class and also add a function for ajax api calls. The client can thereby act as a proxy object, and inside the client we can decide whether we use own written requests or use the backbone webdav dav/sync.

Backbone models should have an id. In theory the real ID is actually the Webdav HREF, not the file's file id (which we can store in a separate property). Path and name can be derived from the href.
Renaming or moving a file is basically changing its Webdav id even though the internal id stays the same. I'll see if it makes sense to fit it this way into the JS code.

Using the webdav href property in the js would probably not make sense, because we don't have this information in the share views. There will not be a need to identify a model system wide, but client wide. We just need an identifier to able to get the correct model from the collection. I think the fileid is an appropriate value.

[mjobst-necls]

@PVince81: I'm looking forward to your comments.

[PVince81]

I can tell you - there is really a lot of work to do.

Indeed. In this iteration the goal is to separate the model/collection from the FileList view better.

While adding the all the functions to the OCA.Files.FileList, I saw a lot of function that are marked as deprecated. How long do these function have to exist?

If possible I'd kill them as soon as possible. Currently I'm not sure yet how many other bundled apps are using them so I left them. But we can aim to remove them within this PR before merging.

The collection attribute of OCA.Files.FileList is only dependent on the model. We should remove that attribute and get the collection out of the model.

Sounds good.

The new button should be a separate object of type OCA.Files.NewFileButton.

Indeed, it shouldl get its separate view. This is even something that could be done first in a separate PR as it doesn't affect the file list directly.

The class OCA.Files.FileList contains properties like breadcrumb, newbutton, _selectedCollection, _selectionSummary, _lastChecked, _dragOptions, _folderDropOptions, _uploader that are not used by OCA.Files.FavoriteFileList, OCA.Sharing.FileList, and OCA.Trashbin.FileList. We could create a new OCA.Files.FileListBase that only provides general behavior.

Yeah, ideal would be to move a lot to the outside and only losely link them together through events.
Regarding FileListBase we could do that as well, yes, considering that it is the base view for all others.

Because the OCA.Files.FileList is a view, it should not have a filesClient attribute, but be able to create a new instance. The filesClient is used in OCA.Files.FileList with its functions

Ideally I'd remove filesClient completely from the FileList and only have it inside the models where all the server requests are supposed to take place. And since the model/collection gets injected from the outside eventually, the FilesClient would then be instantiated outside of FileList too.

reload => filesClient.getFolderContents(): reload() should partially be a part of the model

reload must use the model/collection's fetch which internally could use filesClient.getFolderContents(). Please note that there is currently an alternative to FilesClient and that is using the Backbone-Webdav adapter (which appears as "davSync" within the models) which connects directly to the DAV endpoint without FilesClient. This "davSync" is more recent than FilesClient.

createFile() = > filesClient.putFileContents(): createFile() should partially be a part of the collection/model? (not sure at all, but a file is added to collection and not to a model, so I think it should be a part of the collection)

Part of collection sounds good.

createDirectory() => filesClient.createDirectory(): createDirectory() should be treated like createFile()

Yes, collection.createDirectory().

getPropfindProperties: filesClient.getPropfindProperties() does not need an instance of OC.Files.Client so that it can be used like a static function.

This is the part where I struggle a bit. In the past it was the FileList that specified which properties it required, and that one got extended by plugins like tagsplugins. But now in this PR I'd like to move the PROPFIND to the model's fetch. It is currently possible to call model.fetch({davProperties: [...]}) already, but the part that is not clear is where should the plugins be able to hook themselves to extend the properties list to request for every list.

I propose that we keep the OC.Files.Client class and also add a function for ajax api calls. The client can thereby act as a proxy object, and inside the client we can decide whether we use own written requests or use the backbone webdav dav/sync.

Not sure if a proxy object would be useful here. We already have backbone Webdav sync which is more an adapter than a proxy. It's just that Files Client is a more functional way of requesting properties while backbone is more object oriented.

Using the webdav href property in the js would probably not make sense, because we don't have this information in the share views. There will not be a need to identify a model system wide, but client wide. We just need an identifier to able to get the correct model from the collection. I think the fileid is an appropriate value.

Indeed, that's a good argument. Thanks.

@mjobst-necls thanks a lot for your feedback and interest 😄

[PVince81]

If we can solve the remaining todos from #25909 (comment) and if everything still works including the sub-class lists, then we could in theory merge this already.

One challenge as said before is that currently the "webdav properties selection through plugins" is broken, so some information is missing.

• where to extend/set webdav properties for/from plugins

[PVince81]

Will likely require #26842 to be able to use MKCOL from the directory model to create new directories.

[PVince81]

or maybe we just redesign and rewrite the whole files app from scratch instead of doing gymnastics with the old code...

[PVince81]

this won't work out, the current code is too messy to be able to iteratively improve it.

also I'm having doubts whether Backbone is the right solution here.

there's a mock up with Vue JS here which might be more suited once we wire it up: https://central.owncloud.org/t/gsoc18-project-proposal-new-files-app-written-in-vue-js/11569