From ea047e3201f4f8e899155303bf27d8b1983509f7 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/4] Fix indentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/files_trashbin/js/app.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js index 7cdc157fe4773..ae4a9b1841f7a 100644 --- a/apps/files_trashbin/js/app.js +++ b/apps/files_trashbin/js/app.js @@ -31,17 +31,17 @@ OCA.Trashbin.App = { scrollTo: urlParams.scrollto, config: OCA.Files.App.getFilesConfig(), multiSelectMenu: [ - { - name: 'restore', - displayName: t('files', 'Restore'), - iconClass: 'icon-history', - }, - { - name: 'delete', - displayName: t('files', 'Delete'), - iconClass: 'icon-delete', - } - ] + { + name: 'restore', + displayName: t('files', 'Restore'), + iconClass: 'icon-history', + }, + { + name: 'delete', + displayName: t('files', 'Delete'), + iconClass: 'icon-delete', + } + ] } ); }, From 73125667d4bbfca281e86b6e2d30364660682e06 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 2/4] 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 7294ef9461cd8..3ceadca826d95 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 8fb8a02181127..42b80189287f8 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -226,6 +226,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 524b556a517e8..c1b013ef1b85c 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 750f66236ae3d..b6ca71e15d111 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 ae4a9b1841f7a..b345d876997e5 100644 --- a/apps/files_trashbin/js/app.js +++ b/apps/files_trashbin/js/app.js @@ -41,7 +41,11 @@ OCA.Trashbin.App = { displayName: t('files', 'Delete'), iconClass: 'icon-delete', } - ] + ], + // 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 04ac53d3b3247..2ef8856452831 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 9801b5af363aa0cc9f8d947709b04901f7bc0b6e 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 3/4] 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 8c9c125d0a173..44adf5d7b5b83 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 3a6de0d5012d5..c7a4d2b6d1156 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 c1e37bb387471665e1cdc7096fe1773a206d76c2 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 4/4] 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 00f09900d3e66..3bded3fef11e8 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 bc225e3f9b152..6a39d7a999f74 100644 --- a/tests/acceptance/features/bootstrap/FileListContext.php +++ b/tests/acceptance/features/bootstrap/FileListContext.php @@ -254,6 +254,13 @@ public static function viewFileInFolderMenuItem() { 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 @@ public function iViewInFolder($fileName) { $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 */