Skip to content

Commit 80bcc2d

Browse files
committed
prompt the user when moving and a file exists
test plan: - enable new files - have a multiple files with the same name in different folders - try to drag a file from one of these folders to another and confirm you are prompted to rename or overwrite - test renaming to another name that already exists, and confirm the dialog comes up again - test overwriting when the destination folder has been looked at previously without reloading the page, and confirm no duplicate file appears there - confirm that an error message is displayed if you try to rename a file to an existing name (and the file retains its original name) fixes CNVS-18537 Change-Id: Id5ecedaafcfa762481e65ec2adf61640da5603d8 Reviewed-on: https://gerrit.instructure.com/48679 QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com> Reviewed-by: Dan Minkevitch <dan@instructure.com> Tested-by: Jenkins Product-Review: Jeremy Stanley <jeremy@instructure.com>
1 parent bec4472 commit 80bcc2d

6 files changed

Lines changed: 59 additions & 11 deletions

File tree

app/coffeescripts/models/FilesystemObject.coffee

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,21 @@ define [
1313
displayName: ->
1414
@get('display_name') or @get('name')
1515

16-
moveTo: (newFolder) ->
17-
# only update the new parent_folder_id property
18-
@save({}, {attrs: {parent_folder_id: newFolder.id} }).then =>
16+
moveTo: (newFolder, options = {}) ->
17+
attrs = {parent_folder_id: newFolder.id}
18+
if options.dup == 'overwrite'
19+
$.extend(attrs, {on_duplicate: 'overwrite'})
20+
else if options.dup == 'rename'
21+
if options.name
22+
$.extend(attrs, {name: options.name})
23+
else
24+
$.extend(attrs, {on_duplicate: 'rename'})
25+
@save({}, {attrs: attrs}).then =>
1926
@collection.remove(this)
2027

2128
# add it to newFolder's children
2229
myType = if @saveFrd then 'file' else 'folder' #TODO find a better way to infer type
23-
newFolder[myType+'s'].add(this, {merge:true})
30+
collection = newFolder[myType+'s']
31+
if options.dup == 'overwrite' # remove the overwriten object from the collection
32+
collection.remove(collection.where({display_name: this.get('display_name')}))
33+
collection.add(this, {merge:true})

app/coffeescripts/react_files/components/DialogAdapter.coffee

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ define [
7878
componentDidMount: ->
7979
@node = @getDOMNode()
8080

81-
options =
81+
options =
8282
modal: @props.modal
8383
close: @props.onClose
8484
open: @props.onOpen
@@ -89,5 +89,8 @@ define [
8989
@dialog = $(@node).dialog(options).data('dialog')
9090
@handlePropsChanged()
9191

92+
close: ->
93+
@dialog.close()
94+
9295
render: withReactDOM ->
9396
div {}

app/coffeescripts/react_files/components/FileRenameForm.coffee

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ define [
3636
# pass back expandZip to preserve options that was possibly already made
3737
# in a previous modal
3838
handleReplaceClick: ->
39+
@refs.dialogAdapter.close() if @props.closeOnResolve
3940
@props.onNameConflictResolved({
4041
file: @state.fileOptions.file
4142
dup: 'overwrite'
@@ -45,6 +46,7 @@ define [
4546
# pass back expandZip to preserve options that was possibly already made
4647
# in a previous modal
4748
handleChangeClick: ->
49+
@refs.dialogAdapter.close() if @props.closeOnResolve
4850
@props.onNameConflictResolved({
4951
file: @state.fileOptions.file
5052
dup: 'rename'
@@ -106,7 +108,7 @@ define [
106108

107109

108110
render: withReactDOM ->
109-
DialogAdapter open: @props.fileOptions?, title: I18n.t('rename_title', 'Copy'), onClose: @props.onClose,
111+
DialogAdapter ref: 'dialogAdapter', open: @props.fileOptions?, title: I18n.t('rename_title', 'Copy'), onClose: @props.onClose,
110112
DialogContent {},
111113
@buildContent()
112114
DialogButtons {},

app/coffeescripts/react_files/components/FolderChild.coffee

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,14 @@ define [
6060

6161
saveNameEdit: ->
6262
@setState editing: false, @focusNameLink
63-
@props.model.save(name: @refs.newName.getDOMNode().value, {success: =>
63+
newName = @refs.newName.getDOMNode().value
64+
@props.model.save(name: newName, {
65+
success: =>
6466
@focusNameLink()
65-
})
67+
error: (model, response) =>
68+
$.flashError(I18n.t("A file named %{itemName} already exists in this folder.", itemName: newName)) if response.status == 409
69+
}
70+
)
6671

6772
cancelEditingName: ->
6873
@props.model.collection.remove(@props.model) if @props.model.isNew()

app/coffeescripts/react_files/utils/moveStuff.coffee

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,38 @@
11
define [
22
'i18n!react_files'
33
'jquery'
4-
], (I18n, $) ->
4+
'../components/FileRenameForm'
5+
'old_unsupported_dont_use_react'
6+
'../modules/FileOptionsCollection'
7+
], (I18n, $, FileRenameForm, React, FileOptionsCollection) ->
8+
9+
moveItem = (item, destinationFolder, options = {}) ->
10+
dfd = $.Deferred()
11+
item.moveTo(destinationFolder, options).then(
12+
# success
13+
(data) => dfd.resolve(data)
14+
# failure
15+
(jqXHR, textStatus, errorThrown) =>
16+
if jqXHR.status == 409
17+
# file already exists: prompt and retry
18+
React.renderComponent(FileRenameForm(
19+
closeOnResolve: true
20+
fileOptions: {name: item.attributes.display_name}
21+
onNameConflictResolved: (options) =>
22+
moveItem(item, destinationFolder, options).then(
23+
(data) => dfd.resolve(data)
24+
=> dfd.reject()
25+
)
26+
), $('<div>').appendTo('body')[0])
27+
else
28+
# some other error: fail
29+
dfd.reject()
30+
)
31+
dfd
32+
533

634
moveStuff = (filesAndFolders, destinationFolder) ->
7-
promises = filesAndFolders.map (item) => item.moveTo(destinationFolder)
35+
promises = filesAndFolders.map (item) => moveItem(item, destinationFolder)
836
$.when(promises...).then =>
937
$.flashMessage(I18n.t('move_success', {
1038
one: "%{item} moved to %{destinationFolder}",

public/javascripts/ajax_errors.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ define([
111111
var message = htmlEscape(I18n.t('errors.logged_out', "You are not currently logged in, possibly due to a long period of inactivity."))
112112
message += "<br\/><a href='/login' target='_new'>" + htmlEscape(I18n.t('links.login', 'Login')) + "<\/a>";
113113
$.flashError({ html: message }, 30000);
114-
} else {
114+
} else if (status != 409) {
115115
ajaxErrorFlash(I18n.t('errors.unhandled', "Oops! The last request didn't work out."), request);
116116
}
117117
}, function() {

0 commit comments

Comments
 (0)