Fix attachment renaming

Fixes #822
This commit is contained in:
Dan Stillman 2015-08-07 15:36:46 -04:00
parent 06867d886e
commit cbf4876173
6 changed files with 96 additions and 34 deletions

View File

@ -135,7 +135,35 @@
</property> </property>
<!-- Private properties --> <!-- Methods -->
<constructor>
<![CDATA[
this._notifierID = Zotero.Notifier.registerObserver(this, ['item'], 'attachmentbox');
]]>
</constructor>
<destructor>
<![CDATA[
Zotero.Notifier.unregisterObserver(this._notifierID);
]]>
</destructor>
<method name="notify">
<parameter name="event"/>
<parameter name="type"/>
<parameter name="ids"/>
<body><![CDATA[
if (event != 'modify' || !this.item || !this.item.id) return;
for (let i = 0; i < ids.length; i++) {
if (ids[i] != this.item.id) {
continue;
}
this.refresh();
break;
}
]]></body>
</method>
<method name="refresh"> <method name="refresh">
<body><![CDATA[ <body><![CDATA[

View File

@ -2434,7 +2434,7 @@ Zotero.Item.prototype.fileExistsCached = function () {
* false Attachment file not found * false Attachment file not found
*/ */
Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function* (newName, overwrite) { Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function* (newName, overwrite) {
var origPath = yield this.getFilePath(); var origPath = yield this.getFilePathAsync();
if (!origPath) { if (!origPath) {
Zotero.debug("Attachment file not found in renameAttachmentFile()", 2); Zotero.debug("Attachment file not found in renameAttachmentFile()", 2);
return false; return false;
@ -2442,20 +2442,19 @@ Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function*
try { try {
var origName = OS.Path.basename(origPath); var origName = OS.Path.basename(origPath);
var origModDate = yield OS.File.stat(origPath).lastModificationDate; var origModDate = (yield OS.File.stat(origPath)).lastModificationDate;
newName = Zotero.File.getValidFileName(newName); newName = Zotero.File.getValidFileName(newName);
var destPath = OS.Path.join(OS.Path.dirname(origPath), newName);
var destName = OS.Path.basename(destPath);
// Ignore if no change // Ignore if no change
// if (origName === newName) {
// Note: Just comparing origName to newName isn't reliable Zotero.debug("Filename has not changed");
if (origFileName === destName) {
return true; return true;
} }
var destPath = OS.Path.join(OS.Path.dirname(origPath), newName);
var destName = OS.Path.basename(destPath);
// Update mod time and clear hash so the file syncs // Update mod time and clear hash so the file syncs
// TODO: use an integer counter instead of mod time for change detection // TODO: use an integer counter instead of mod time for change detection
// Update mod time first, because it may fail for read-only files on Windows // Update mod time first, because it may fail for read-only files on Windows
@ -2475,8 +2474,8 @@ Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function*
yield this.relinkAttachmentFile(destPath); yield this.relinkAttachmentFile(destPath);
yield Zotero.DB.executeTransaction(function* () { yield Zotero.DB.executeTransaction(function* () {
Zotero.Sync.Storage.setSyncedHash(this.id, null, false); yield Zotero.Sync.Storage.setSyncedHash(this.id, null, false);
Zotero.Sync.Storage.setSyncState(this.id, Zotero.Sync.Storage.SYNC_STATE_TO_UPLOAD); yield Zotero.Sync.Storage.setSyncState(this.id, Zotero.Sync.Storage.SYNC_STATE_TO_UPLOAD);
}.bind(this)); }.bind(this));
return true; return true;
@ -2529,7 +2528,7 @@ Zotero.Item.prototype.relinkAttachmentFile = Zotero.Promise.coroutine(function*
try { try {
// If selected file isn't in the attachment's storage directory, // If selected file isn't in the attachment's storage directory,
// copy it in and use that one instead // copy it in and use that one instead
var storageDir = Zotero.Attachments.getStorageDirectory(this.id).path; var storageDir = Zotero.Attachments.getStorageDirectory(this).path;
if (this.isImportedAttachment() && OS.Path.dirname(path) != storageDir) { if (this.isImportedAttachment() && OS.Path.dirname(path) != storageDir) {
// If file with same name already exists in the storage directory, // If file with same name already exists in the storage directory,
// move it out of the way // move it out of the way
@ -2560,7 +2559,7 @@ Zotero.Item.prototype.relinkAttachmentFile = Zotero.Promise.coroutine(function*
this.attachmentPath = Zotero.Attachments.getPath(Zotero.File.pathToFile(newPath), linkMode); this.attachmentPath = Zotero.Attachments.getPath(Zotero.File.pathToFile(newPath), linkMode);
yield this.save({ yield this.saveTx({
skipDateModifiedUpdate: true, skipDateModifiedUpdate: true,
skipClientDateModifiedUpdate: skipItemUpdate skipClientDateModifiedUpdate: skipItemUpdate
}); });

View File

@ -499,7 +499,7 @@ Zotero.Sync.Storage = new function () {
*/ */
this.getSyncState = function (itemID) { this.getSyncState = function (itemID) {
var sql = "SELECT syncState FROM itemAttachments WHERE itemID=?"; var sql = "SELECT syncState FROM itemAttachments WHERE itemID=?";
return Zotero.DB.valueQuery(sql, itemID); return Zotero.DB.valueQueryAsync(sql, itemID);
} }
@ -507,7 +507,7 @@ Zotero.Sync.Storage = new function () {
* @param {Integer} itemID * @param {Integer} itemID
* @param {Integer} syncState Constant from Zotero.Sync.Storage * @param {Integer} syncState Constant from Zotero.Sync.Storage
*/ */
this.setSyncState = function (itemID, syncState) { this.setSyncState = Zotero.Promise.method(function (itemID, syncState) {
switch (syncState) { switch (syncState) {
case this.SYNC_STATE_TO_UPLOAD: case this.SYNC_STATE_TO_UPLOAD:
case this.SYNC_STATE_TO_DOWNLOAD: case this.SYNC_STATE_TO_DOWNLOAD:
@ -517,13 +517,12 @@ Zotero.Sync.Storage = new function () {
break; break;
default: default:
throw "Invalid sync state '" + syncState throw new Error("Invalid sync state '" + syncState);
+ "' in Zotero.Sync.Storage.setSyncState()"
} }
var sql = "UPDATE itemAttachments SET syncState=? WHERE itemID=?"; var sql = "UPDATE itemAttachments SET syncState=? WHERE itemID=?";
return Zotero.DB.valueQuery(sql, [syncState, itemID]); return Zotero.DB.valueQueryAsync(sql, [syncState, itemID]);
} });
/** /**
@ -572,18 +571,18 @@ Zotero.Sync.Storage = new function () {
/** /**
* @param {Integer} itemID * @param {Integer} itemID
* @return {String|NULL} File hash, or NULL if never synced * @return {Promise<String|null|false>} - File hash, null if never synced, if false if
* file doesn't exist
*/ */
this.getSyncedHash = function (itemID) { this.getSyncedHash = Zotero.Promise.coroutine(function* (itemID) {
var sql = "SELECT storageHash FROM itemAttachments WHERE itemID=?"; var sql = "SELECT storageHash FROM itemAttachments WHERE itemID=?";
var hash = Zotero.DB.valueQuery(sql, itemID); var hash = yield Zotero.DB.valueQueryAsync(sql, itemID);
if (hash === false) { if (hash === false) {
throw "Item " + itemID + " not found in " throw new Error("Item " + itemID + " not found");
+ "Zotero.Sync.Storage.getSyncedHash()";
} }
return hash; return hash;
} })
/** /**
@ -592,24 +591,22 @@ Zotero.Sync.Storage = new function () {
* @param {Boolean} [updateItem=FALSE] Update dateModified field of * @param {Boolean} [updateItem=FALSE] Update dateModified field of
* attachment item * attachment item
*/ */
this.setSyncedHash = function (itemID, hash, updateItem) { this.setSyncedHash = Zotero.Promise.coroutine(function* (itemID, hash, updateItem) {
if (hash !== null && hash.length != 32) { if (hash !== null && hash.length != 32) {
throw ("Invalid file hash '" + hash + "' in Zotero.Storage.setSyncedHash()"); throw ("Invalid file hash '" + hash + "' in Zotero.Storage.setSyncedHash()");
} }
Zotero.DB.beginTransaction(); Zotero.DB.requireTransaction();
var sql = "UPDATE itemAttachments SET storageHash=? WHERE itemID=?"; var sql = "UPDATE itemAttachments SET storageHash=? WHERE itemID=?";
Zotero.DB.valueQuery(sql, [hash, itemID]); yield Zotero.DB.queryAsync(sql, [hash, itemID]);
if (updateItem) { if (updateItem) {
// Update item date modified so the new mod time will be synced // Update item date modified so the new mod time will be synced
var sql = "UPDATE items SET clientDateModified=? WHERE itemID=?"; var sql = "UPDATE items SET clientDateModified=? WHERE itemID=?";
Zotero.DB.query(sql, [Zotero.DB.transactionDateTime, itemID]); yield Zotero.DB.queryAsync(sql, [Zotero.DB.transactionDateTime, itemID]);
} }
});
Zotero.DB.commitTransaction();
}
/** /**

View File

@ -4104,7 +4104,7 @@ var ZoteroPane = new function()
newName = newName + ext; newName = newName + ext;
} }
var renamed = item.renameAttachmentFile(newName); var renamed = yield item.renameAttachmentFile(newName);
if (renamed !== true) { if (renamed !== true) {
Zotero.debug("Could not rename file (" + renamed + ")"); Zotero.debug("Could not rename file (" + renamed + ")");
continue; continue;

View File

@ -29,6 +29,22 @@ describe("Item pane", function () {
}) })
}) })
describe("Attachment pane", function () {
it("should refresh on file rename", function* () {
var file = getTestDataDirectory();
file.append('test.png');
var item = yield Zotero.Attachments.importFromFile({
file: file
});
var newName = 'test2.png';
yield item.renameAttachmentFile(newName);
var itemBox = doc.getElementById('zotero-attachment-box');
var label = itemBox._id('fileName');
assert.equal(label.value, newName);
})
})
describe("Note pane", function () { describe("Note pane", function () {
it("should refresh on note update", function* () { it("should refresh on note update", function* () {
var item = new Zotero.Item('note'); var item = new Zotero.Item('note');

View File

@ -539,6 +539,28 @@ describe("Zotero.Item", function () {
}); });
}) })
describe("#renameAttachmentFile()", function () {
it("should rename an attached file", function* () {
var file = getTestDataDirectory();
file.append('test.png');
var item = yield Zotero.Attachments.importFromFile({
file: file
});
var newName = 'test2.png';
yield item.renameAttachmentFile(newName);
assert.equal(item.attachmentFilename, newName);
var path = yield item.getFilePathAsync();
assert.equal(OS.Path.basename(path), newName)
yield OS.File.exists(path);
assert.equal(
(yield Zotero.Sync.Storage.getSyncState(item.id)),
Zotero.Sync.Storage.SYNC_STATE_TO_UPLOAD
);
assert.isNull(yield Zotero.Sync.Storage.getSyncedHash(item.id));
})
})
describe("#setTags", function () { describe("#setTags", function () {
it("should save an array of tags in API JSON format", function* () { it("should save an array of tags in API JSON format", function* () {
var tags = [ var tags = [