Bug 99744 - Add "Save as" to cloud version
Summary: Add "Save as" to cloud version
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice Online
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-09 12:41 UTC by Thomas Krumbein
Modified: 2017-11-27 09:53 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Krumbein 2016-05-09 12:41:21 UTC
In actual cloud version (CODE) it is opssible, to create new documents (writer, calc, impress). That works fine, but it is impossible to give those new documents unique names. They will always been named automotic as "New document(xx)", "New Spreedsheet(xx)" and so on.

After creation and storing the document, it is possible to rename the file inside the cloud filesystem (i.e. ownCloud documents). 

It should be possible to rename the the document inside LibreOffice online - maybe with a menu-point "save as" or under file-menu adding a entry "save as".

that make even testing easier :)
Comment 1 Pranav Kant 2017-05-18 08:23:57 UTC
(In reply to Thomas Krumbein from comment #0)
> In actual cloud version (CODE) it is opssible, to create new documents
> (writer, calc, impress). That works fine, but it is impossible to give those
> new documents unique names. They will always been named automotic as "New
> document(xx)", "New Spreedsheet(xx)" and so on.

The new owncloud/nextcloud has this drop down menu that creates new document and allows you to name new documents too. 

> 
> After creation and storing the document, it is possible to rename the file
> inside the cloud filesystem (i.e. ownCloud documents). 

Yeah, this would be good to have.

> 
> It should be possible to rename the the document inside LibreOffice online -
> maybe with a menu-point "save as" or under file-menu adding a entry "save
> as".
> 
> that make even testing easier :)
Comment 2 Aditya Dewan 2017-05-24 08:22:01 UTC
If we wish to implement this feature, the series of events would be a bit similar to that of the save feature, as implemented currently.

As per my understanding, this is what was happening when a save request was sent:

- on a save request
    DocumentBroker::sendUnoSave
    -
    DocumentBroker::forwardtotochild, sent to lokit
    -
    *very less idea what happens here, any guidance would be helpful here*
    -
    ClientSession::handleKitToClientMessage, we get response of uno command
    -
    check members and call DocumentBroker::savetostorage

- on a saveas request
    request recieved from click/shortuct press
    DocumentBroker -> send uno saveas request
        we write request
        and forward to kit
    -
    call function to save jailed file to the new specified location 
        we write a new method for this that will save the jailed file to a new location and now documentbroker's uri.
    -
    then, we need to make a new DocumentBroker, for the current session.
    -
    remove current session from previous docbroker and attach it to the new one.
    -
    call save function from the new document broker.
    (would that be it?)


*some code was already written regarding handling messages related to Saveas from the lokit to the clientsession.(clientsession.cpp, l.no. - 573), but all it does is creates the uri corresponding to the new path, sends the file(why?), removes the client session, and signals the docbroker to exit.
The last part makes this confusing though,lets say 3 users a, b, c have opened the same document, now if 'a'saves it as some other file, then we need not shift 'b' and 'c'.(or do we?)
Comment 3 Pranav Kant 2017-05-31 08:17:50 UTC
(In reply to Aditya Dewan from comment #2)
> If we wish to implement this feature, the series of events would be a bit
> similar to that of the save feature, as implemented currently.
> 
> As per my understanding, this is what was happening when a save request was
> sent:
> 
> - on a save request
>     DocumentBroker::sendUnoSave
>     -
>     DocumentBroker::forwardtotochild, sent to lokit
>     -
>     *very less idea what happens here, any guidance would be helpful here*

You mean what happens inside LO core when save is dispatched to it ? Basically, the call ends up in doc_saveAs method in desktop/source/lib/init.cxx and it eventually saves the document (and notify optionally).

>     -
>     ClientSession::handleKitToClientMessage, we get response of uno command
>     -
>     check members and call DocumentBroker::savetostorage
> 

Yeah, that's correct.

> - on a saveas request
>     request recieved from click/shortuct press
>     DocumentBroker -> send uno saveas request
>         we write request
>         and forward to kit
>     -
>     call function to save jailed file to the new specified location 
>         we write a new method for this that will save the jailed file to a
> new location and now documentbroker's uri.
>     -
>     then, we need to make a new DocumentBroker, for the current session.
>     -
>     remove current session from previous docbroker and attach it to the new
> one.
>     -
>     call save function from the new document broker.
>     (would that be it?)
> 
> 
> *some code was already written regarding handling messages related to Saveas
> from the lokit to the clientsession.(clientsession.cpp, l.no. - 573), but
> all it does is creates the uri corresponding to the new path, sends the
> file(why?), removes the client session, and signals the docbroker to exit.

The current saveas implementation is only used for HTTP POST conversion API. See wsd/reference.txt for "Document conversion". Since, the post request is atomic, it just downloads the file after which DocumentBroker is not needed, so we remove it.

Surely, you need to do some modifications here to preclude the document broker to exit in this case.

> The last part makes this confusing though,lets say 3 users a, b, c have
> opened the same document, now if 'a' saves it as some other file, then we
> need not shift 'b' and 'c'.(or do we?)

No, we shouldn't shift 'b' and 'c'. Only 'a' should be shifted.

Please note that, this is 'Save as' to "cloud" versions, so it also needs to be stored back to the cloud storage via WOPI. You need to create new document in the cloud storage when user hits the 'save as' button.

In loleaflet, there is a saveAs method in Toolbar.js which is unused right now. But it seems there were efforts made in the past to 'Save as' while 'Taking ownership' at the same time, hence the TakeOwnership flag there. It would be great to experiment a bit with it to see if it works out of the box, or needs some tweaking. Personally, I'd take a top-down approach here and start with the WOPI storage, create a dialog box there that asks me the location of the file etc. to save as, document format and then sends appropriate commands back to loolwsd.
Comment 4 Commit Notification 2017-10-25 20:23:26 UTC
Pranav Kant committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=6e86fefc4eba5546096f4c4d974ffc65d11570ae

tdf#99744: Check if params exist before accessing them
Comment 5 Commit Notification 2017-10-26 09:15:13 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=f4198526cad3861582f6a56a82c3a91e696ab391

tdf#99744 SaveAs: Reverts parts of the previous Save As work.
Comment 6 Commit Notification 2017-10-26 09:15:21 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=7ff432a37010031afd3da4012882b160aba51ec9

tdf#99744 SaveAs: Reimplementation of the PutRelativeFile going through Kit.
Comment 7 Commit Notification 2017-10-26 09:15:28 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=6fe44843954bb68f8b958559b1b0ff37b945cf80

tdf#99744 SaveAs: Use X-WOPI-SuggestedTarget instead of X-WOPI-RelativeTarget.
Comment 8 Commit Notification 2017-10-26 09:15:38 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=6745464c70b1adf4c0d523ebe4afe3feed814cac

tdf#99744 SaveAs: Report back to loleaflet that the saveas succeeded.
Comment 9 Commit Notification 2017-10-26 09:15:50 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=343c5bc69083c844b7896d7a62b40a44d8b06b03

tdf#99744 SaveAs: Extend test to check that the Save As result was sent.
Comment 10 Xisco Faulí 2017-11-26 17:48:26 UTC
A polite ping to Jan Holesovsky: is this bug fixed? if so, could you
please close it as RESOLVED FIXED ? Thanks
Comment 11 Jan Holesovsky 2017-11-27 09:53:52 UTC
Ah yes, indeed it is fixed on the Online side; but of course depends on the integration side if they implement their part of this or not (but several already do).