From 4df1ebeeaf22251a81ed413cb201b2d554c14dff Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 17 Feb 2021 14:48:39 +0100 Subject: [PATCH 1/4] Fix issues with duplicated file names in the same directory --- apps/files/js/fileactions.js | 1 + apps/files/js/filelist.js | 33 +++++++++++++++++-- changelog/unreleased/38415 | 8 +++++ .../shareByPublickLinkOc10Issue35177.feature | 3 +- 4 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 changelog/unreleased/38415 diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 22049cd2c895..51a84c7f9373 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -434,6 +434,7 @@ a: null }, function(event) { + context.fileList.$fileList.trigger(new jQuery.Event('setCurrentRow', {currentRow: context.$file})); event.stopPropagation(); event.preventDefault(); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 6f177131148a..70f3285da1b7 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -91,6 +91,11 @@ */ initialized: false, + /** + * Last clicked row + */ + $currentRow: null, + /** * Number of files per page * @@ -325,6 +330,7 @@ this.updateSearch(); + this.$fileList.on('setCurrentRow', _.bind(this._setCurrentRow, this)); this.$fileList.on('click','td.filename>a.name, td.filesize, td.date', _.bind(this._onClickFile, this)); this.$fileList.on('change', 'td.filename>.selectCheckBox', _.bind(this._onClickFileCheckbox, this)); @@ -396,7 +402,7 @@ this.$el.off('urlChanged.filelistbound'); // remove events attached to the $container this.$container.off('scroll.' + this.$el.attr('id')); - + this.$fileList.off('setCurrentRow'); }, /** @@ -453,7 +459,10 @@ } // if requesting the selected model, return it - if (this._currentFileModel && this._currentFileModel.get('name') === fileName) { + if (this._currentFileModel && + this._currentFileModel.get('name') === fileName && + (!this.$currentRow || this.$currentRow.data('id') === this._currentFileModel.id) + ) { return this._currentFileModel; } @@ -630,6 +639,9 @@ * Event handler for when clicking on files to select them */ _onClickFile: function(event) { + var currentRow = $(event.currentTarget).closest('tr'); + this.$fileList.trigger(new jQuery.Event('setCurrentRow', {currentRow: currentRow})); + var $link = $(event.target).closest('a'); if ($link.attr('href') === '#' || $link.hasClass('disable-click')) { event.preventDefault(); @@ -930,7 +942,13 @@ */ findFileEl: function(fileName){ // use filterAttr to avoid escaping issues - return this.$fileList.find('tr').filterAttr('data-file', fileName); + var $fileEl = this.$fileList.find('tr').filterAttr('data-file', fileName); + + if (this.$currentRow && $fileEl.length > 1) { + $fileEl = $fileEl.filter('[data-id="' + this.$currentRow.data('id') + '"]'); + } + + return $fileEl; }, /** @@ -1566,6 +1584,15 @@ } this.breadcrumb.setDirectory(this.getCurrentDirectory()); }, + + /** + * Sets the currently clicked row + * We use this event to give the findFileEl a unique identifier. + */ + _setCurrentRow: function(data) { + this.$currentRow = data.currentRow; + }, + /** * Sets the current sorting and refreshes the list * diff --git a/changelog/unreleased/38415 b/changelog/unreleased/38415 new file mode 100644 index 000000000000..d0263da71639 --- /dev/null +++ b/changelog/unreleased/38415 @@ -0,0 +1,8 @@ +Bugfix: Fix issues with duplicated file names in the same directory + +In some views like the "Shared by link"-list it is possible to have +one or more files with the same name in one directory. This fix corrects +plenty of wrong behaviors that such a scenario caused in the UI. + +https://github.com/owncloud/core/pull/38415 +https://github.com/owncloud/enterprise/issues/4412 diff --git a/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature b/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature index a098dfd752a4..534e4ddb4425 100644 --- a/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature +++ b/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature @@ -29,6 +29,5 @@ Feature: Share by public link And the user has browsed to the shared-by-link page When the user renames folder "newfolder" to "newfolder1" using the webUI Then folder "newfolder1" should be listed on the webUI - And folder "newfolder" should not be listed on the webUI - #And folder "newfolder" should be listed on the webUI + And folder "newfolder" should be listed on the webUI And folder "test" should be listed on the webUI From e373edca8ac506c0a0af3b45d812c41dc9b3777d Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 18 Feb 2021 06:13:55 +0545 Subject: [PATCH 2/4] Delete bug demo scenario and enable good test in oC10 --- .../shareByPublicLink.feature | 2 +- .../shareByPublickLinkOc10Issue35177.feature | 33 ------------------- 2 files changed, 1 insertion(+), 34 deletions(-) delete mode 100644 tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature diff --git a/tests/acceptance/features/webUISharingPublic2/shareByPublicLink.feature b/tests/acceptance/features/webUISharingPublic2/shareByPublicLink.feature index a9b2106ee79e..fd749ba10aee 100644 --- a/tests/acceptance/features/webUISharingPublic2/shareByPublicLink.feature +++ b/tests/acceptance/features/webUISharingPublic2/shareByPublicLink.feature @@ -184,7 +184,7 @@ Feature: Share by public link And the public accesses the last created public link using the webUI Then the content of the file shared by the last public link should be the same as "lorem.txt" - @skipOnOcV10 @issue-35177 + @skipOnOcV10.5 @skipOnOcV10.6 Scenario: User renames a subfolder among subfolders with same names which are shared by public links Given user "Alice" has created folder "nf1" And user "Alice" has created folder "nf1/newfolder" diff --git a/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature b/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature deleted file mode 100644 index 534e4ddb4425..000000000000 --- a/tests/acceptance/features/webUISharingPublic2/shareByPublickLinkOc10Issue35177.feature +++ /dev/null @@ -1,33 +0,0 @@ -@webUI @notToImplementOnOCIS @insulated @disablePreviews @mailhog @public_link_share-feature-required @files_sharing-app-required -Feature: Share by public link - As a user - I want to share files through a publicly accessible link - So that users who do not have an account on my ownCloud server can access them - - As an admin - I want to limit the ability of a user to share files/folders through a publicly accessible link - So that public sharing is limited according to organization policy - - Background: - Given user "Alice" has been created with default attributes and without skeleton files - - @issue-35177 - Scenario: User renames a subfolder among subfolders with same names which are shared by public links - Given user "Alice" has created folder "nf1" - And user "Alice" has created folder "nf1/newfolder" - And user "Alice" has created folder "nf2" - And user "Alice" has created folder "nf2/newfolder" - And user "Alice" has created folder "test" - And user "Alice" has created folder "test/test" - And user "Alice" has created a public link share with settings - | path | nf1/newfolder | - And user "Alice" has created a public link share with settings - | path | nf2/newfolder | - And user "Alice" has created a public link share with settings - | path | test/test | - And user "Alice" has logged in using the webUI - And the user has browsed to the shared-by-link page - When the user renames folder "newfolder" to "newfolder1" using the webUI - Then folder "newfolder1" should be listed on the webUI - And folder "newfolder" should be listed on the webUI - And folder "test" should be listed on the webUI From 2b990fab8f3c538017858f3f5ba8c345aba5b9c4 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 18 Feb 2021 10:14:51 +0100 Subject: [PATCH 3/4] Add comments and remove unnecessary event call --- apps/files/js/filelist.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 70f3285da1b7..9c2cd165c022 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -459,6 +459,8 @@ } // if requesting the selected model, return it + // also take the file id into account if possible because + // the file name may not be unique if (this._currentFileModel && this._currentFileModel.get('name') === fileName && (!this.$currentRow || this.$currentRow.data('id') === this._currentFileModel.id) @@ -640,7 +642,7 @@ */ _onClickFile: function(event) { var currentRow = $(event.currentTarget).closest('tr'); - this.$fileList.trigger(new jQuery.Event('setCurrentRow', {currentRow: currentRow})); + this._setCurrentRow({currentRow: currentRow}); var $link = $(event.target).closest('a'); if ($link.attr('href') === '#' || $link.hasClass('disable-click')) { @@ -944,6 +946,8 @@ // use filterAttr to avoid escaping issues var $fileEl = this.$fileList.find('tr').filterAttr('data-file', fileName); + // take the file id into account if possible because the file name + // may not be unique which results in multiple elements if (this.$currentRow && $fileEl.length > 1) { $fileEl = $fileEl.filter('[data-id="' + this.$currentRow.data('id') + '"]'); } From d55ed9a8ecc15bd264b352b2f0385501cd9a63ca Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 18 Feb 2021 12:37:27 +0100 Subject: [PATCH 4/4] Remove event handlers completely as they are not necessary --- apps/files/js/fileactions.js | 2 +- apps/files/js/filelist.js | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 51a84c7f9373..96c643d3d0fa 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -434,7 +434,7 @@ a: null }, function(event) { - context.fileList.$fileList.trigger(new jQuery.Event('setCurrentRow', {currentRow: context.$file})); + context.fileList._setCurrentRow(context.$file); event.stopPropagation(); event.preventDefault(); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 9c2cd165c022..40cb4bfb829a 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -330,7 +330,6 @@ this.updateSearch(); - this.$fileList.on('setCurrentRow', _.bind(this._setCurrentRow, this)); this.$fileList.on('click','td.filename>a.name, td.filesize, td.date', _.bind(this._onClickFile, this)); this.$fileList.on('change', 'td.filename>.selectCheckBox', _.bind(this._onClickFileCheckbox, this)); @@ -402,7 +401,6 @@ this.$el.off('urlChanged.filelistbound'); // remove events attached to the $container this.$container.off('scroll.' + this.$el.attr('id')); - this.$fileList.off('setCurrentRow'); }, /** @@ -641,8 +639,7 @@ * Event handler for when clicking on files to select them */ _onClickFile: function(event) { - var currentRow = $(event.currentTarget).closest('tr'); - this._setCurrentRow({currentRow: currentRow}); + this._setCurrentRow($(event.currentTarget).closest('tr')); var $link = $(event.target).closest('a'); if ($link.attr('href') === '#' || $link.hasClass('disable-click')) { @@ -1593,8 +1590,8 @@ * Sets the currently clicked row * We use this event to give the findFileEl a unique identifier. */ - _setCurrentRow: function(data) { - this.$currentRow = data.currentRow; + _setCurrentRow: function($rowEl) { + this.$currentRow = $rowEl; }, /**