From 449349e9010714e55e2667a7a84956720806581a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:32:15 +0200 Subject: [PATCH 1/3] Fix opening a section again in the Files app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a section is open in the Files app a "show" event is triggered. File list objects handle that event by reloading themselves, but only if the file list was shown at least once. However, the file list objects of plugins are created when the "show" event is triggered for the first time for their section; as the file list objects register their handler for the "show" event when they are created they never handle the first triggered "show" event, as the handler is set while that event is being already handled. Therefore, from the point of view of the handler, the second time that a "show" event was triggered it was seen as if the file list was shown for the first time, and thus it was not reloaded. Now the "shown" property is explicitly set for those file lists that are created while handling a "show" event, which causes them to be reloaded as expected when opening their section again. Note that it is not possible to just reload the file list whenever it is shown; the file list is reloaded also when the directory changes, and this can happen when the web page is initially loaded and the URL is parsed. In that case, if file lists were reloaded when shown for the first time then it could be reloaded twice, one with the default parameters due to the "show" event and another one with the proper parameters once the URL was parsed, and the files that appeard in the list would depend on which response from the server was received the last. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/favoritesplugin.js | 5 +++++ apps/files/js/filelist.js | 4 ++++ apps/files/js/recentplugin.js | 5 +++++ apps/files_sharing/js/app.js | 30 +++++++++++++++++++++++++----- apps/files_trashbin/js/app.js | 6 +++++- apps/systemtags/js/app.js | 7 ++++++- 6 files changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/files/js/favoritesplugin.js b/apps/files/js/favoritesplugin.js index 7294ef9461..3ceadca826 100644 --- a/apps/files/js/favoritesplugin.js +++ b/apps/files/js/favoritesplugin.js @@ -67,6 +67,11 @@ return new OCA.Files.FavoritesFileList( $el, { fileActions: fileActions, + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); }, diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 0a6620ab22..a4d306edc1 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -221,6 +221,10 @@ return; } + if (options.shown) { + this.shown = options.shown; + } + if (options.config) { this._filesConfig = options.config; } else if (!_.isUndefined(OCA.Files) && !_.isUndefined(OCA.Files.App)) { diff --git a/apps/files/js/recentplugin.js b/apps/files/js/recentplugin.js index 524b556a51..c1b013ef1b 100644 --- a/apps/files/js/recentplugin.js +++ b/apps/files/js/recentplugin.js @@ -67,6 +67,11 @@ return new OCA.Files.RecentFileList( $el, { fileActions: fileActions, + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); }, diff --git a/apps/files_sharing/js/app.js b/apps/files_sharing/js/app.js index 750f66236a..b6ca71e15d 100644 --- a/apps/files_sharing/js/app.js +++ b/apps/files_sharing/js/app.js @@ -34,7 +34,11 @@ OCA.Sharing.App = { id: 'shares.self', sharedWithUser: true, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -56,7 +60,11 @@ OCA.Sharing.App = { id: 'shares.others', sharedWithUser: false, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -78,7 +86,11 @@ OCA.Sharing.App = { id: 'shares.link', linksOnly: true, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -101,7 +113,11 @@ OCA.Sharing.App = { showDeleted: true, sharedWithUser: true, fileActions: this._restoreShareAction(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -122,7 +138,11 @@ OCA.Sharing.App = { { id: 'shares.overview', config: OCA.Files.App.getFilesConfig(), - isOverview: true + isOverview: true, + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js index 82e47d510b..199cb7d4f8 100644 --- a/apps/files_trashbin/js/app.js +++ b/apps/files_trashbin/js/app.js @@ -51,7 +51,11 @@ OCA.Trashbin.App = { iconClass: 'icon-delete', } ], - client: this.client + client: this.client, + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); }, diff --git a/apps/systemtags/js/app.js b/apps/systemtags/js/app.js index 04ac53d3b3..2ef8856452 100644 --- a/apps/systemtags/js/app.js +++ b/apps/systemtags/js/app.js @@ -28,7 +28,12 @@ { id: 'systemtags', fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); From 0426ba6de5c367a07414a5dbf54e1774d83d8803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:33:05 +0200 Subject: [PATCH 2/3] Remove event handler no longer needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom handler for "URL changed" events were added to reload the file list whenever the sections for favorites and shares were opened; this was used to fix the problem of not reloading the file lists when opening them for a second time. However, besides that the handlers were not really necessary, and as the root of the bug was fixed in the previous commit those handlers are now removed. The file list for tags uses the handler for a different purpose, though, so that one was kept. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/favoritesfilelist.js | 6 ------ apps/files_sharing/js/sharedfilelist.js | 6 ------ 2 files changed, 12 deletions(-) diff --git a/apps/files/js/favoritesfilelist.js b/apps/files/js/favoritesfilelist.js index 8c9c125d0a..44adf5d7b5 100644 --- a/apps/files/js/favoritesfilelist.js +++ b/apps/files/js/favoritesfilelist.js @@ -94,12 +94,6 @@ $(document).ready(function() { return OCA.Files.FileList.prototype.reloadCallback.call(this, status, result); }, - - _onUrlChanged: function (e) { - if (e && _.isString(e.dir)) { - this.changeDirectory(e.dir, false, true); - } - } }); OCA.Files.FavoritesFileList = FavoritesFileList; diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index 19250815bd..1ace9f3cd1 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -444,12 +444,6 @@ // Sort by expected sort comparator return files.sort(this._sortComparator); }, - - _onUrlChanged: function(e) { - if (e && _.isString(e.dir)) { - this.changeDirectory(e.dir, false, true); - } - } }); /** From 779000eb139fb103d1cc3871115739d7f1beb5f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:34:19 +0200 Subject: [PATCH 3/3] Add acceptance tests for opening a section in the Files app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/app-files.feature | 72 +++++++++++++++++++ .../features/bootstrap/FileListContext.php | 16 +++++ 2 files changed, 88 insertions(+) diff --git a/tests/acceptance/features/app-files.feature b/tests/acceptance/features/app-files.feature index 00f09900d3..3bded3fef1 100644 --- a/tests/acceptance/features/app-files.feature +++ b/tests/acceptance/features/app-files.feature @@ -31,6 +31,78 @@ Feature: app-files When I open the details view for "welcome.txt" Then I see that the details view is open + Scenario: show recent files + Given I am logged in + And I create a new folder named "Folder just created" + When I open the "Recent" section + Then I see that the current section is "Recent" + Then I see that the file list contains a file named "Folder just created" + + Scenario: show recent files for a second time + Given I am logged in + And I open the "Recent" section + And I see that the current section is "Recent" + And I open the "All files" section + And I see that the current section is "All files" + And I create a new folder named "Folder just created" + When I open the "Recent" section + Then I see that the current section is "Recent" + Then I see that the file list contains a file named "Folder just created" + + Scenario: show favorites + Given I am logged in + And I mark "welcome.txt" as favorite + When I open the "Favorites" section + Then I see that the current section is "Favorites" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show favorites for a second time + Given I am logged in + And I open the "Favorites" section + And I see that the current section is "Favorites" + And I open the "All files" section + And I see that the current section is "All files" + And I mark "welcome.txt" as favorite + When I open the "Favorites" section + Then I see that the current section is "Favorites" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show shares + Given I am logged in + And I share the link for "welcome.txt" + When I open the "Shares" section + Then I see that the current section is "Shares" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show shares for a second time + Given I am logged in + And I open the "Shares" section + And I see that the current section is "Shares" + And I open the "All files" section + And I see that the current section is "All files" + And I share the link for "welcome.txt" + When I open the "Shares" section + Then I see that the current section is "Shares" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show deleted files + Given I am logged in + And I delete "welcome.txt" + When I open the "Deleted files" section + Then I see that the current section is "Deleted files" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show deleted files for a second time + Given I am logged in + And I open the "Deleted files" section + And I see that the current section is "Deleted files" + And I open the "All files" section + And I see that the current section is "All files" + And I delete "welcome.txt" + When I open the "Deleted files" section + Then I see that the current section is "Deleted files" + Then I see that the file list contains a file named "welcome.txt" + Scenario: rename a file with the details view open Given I am logged in And I open the details view for "welcome.txt" diff --git a/tests/acceptance/features/bootstrap/FileListContext.php b/tests/acceptance/features/bootstrap/FileListContext.php index bc225e3f9b..6a39d7a999 100644 --- a/tests/acceptance/features/bootstrap/FileListContext.php +++ b/tests/acceptance/features/bootstrap/FileListContext.php @@ -254,6 +254,13 @@ class FileListContext implements Context, ActorAwareInterface { return self::fileActionsMenuItemFor("View in folder"); } + /** + * @return Locator + */ + public static function deleteMenuItem() { + return self::fileActionsMenuItemFor("Delete"); + } + /** * @Given I create a new folder named :folderName */ @@ -322,6 +329,15 @@ class FileListContext implements Context, ActorAwareInterface { $this->actor->find(self::viewFileInFolderMenuItem(), 2)->click(); } + /** + * @When I delete :fileName + */ + public function iDelete($fileName) { + $this->actor->find(self::fileActionsMenuButtonForFile($this->fileListAncestor, $fileName), 10)->click(); + + $this->actor->find(self::deleteMenuItem(), 2)->click(); + } + /** * @Then I see that the file list is eventually loaded */