From 1aa82094f16cd0d9f22e73205d961979536d9254 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 21 Mar 2023 06:23:16 -0400 Subject: [PATCH] Better way of skipping migration of Place and Date from Extra Or at least a way that we already have built-in and that only applies to the call in `Zotero.Item::migrateExtraFields()`. This doesn't distinguish between CSL fields (`publisher-place`, `event-place`, `issued`) and actual Zotero field, and we really only need to skip the former, but it's fine. Follow-up to e3cfeee81, related to #3030 --- chrome/content/zotero/xpcom/data/item.js | 11 +++++++- .../zotero/xpcom/utilities_internal.js | 13 +--------- test/tests/schemaTest.js | 14 +++++++++++ test/tests/utilities_internalTest.js | 25 +++---------------- 4 files changed, 28 insertions(+), 35 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 33b4703c3a..34154ba224 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -5442,7 +5442,16 @@ Zotero.Item.prototype.migrateExtraFields = function () { try { var { itemType, fields, creators, extra } = Zotero.Utilities.Internal.extractExtraFields( - originalExtra, this + originalExtra, + this, + [ + // Skip 'publisher-place' and 'event-place' for now, since the mappings will be changed + // https://github.com/citation-style-language/zotero-bits/issues/6 + 'place', + // Skip 'issued' for now, since we don't support date ranges in Date + // https://github.com/zotero/zotero/issues/3030 + 'date' + ] ); if (itemType) { let originalType = this.itemTypeID; diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index bae1880878..075e743296 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -1297,20 +1297,9 @@ Zotero.Utilities.Internal = { return true; } - // Skip for now, since the mappings to Place will be changed - // https://github.com/citation-style-language/zotero-bits/issues/6 - if (key == 'event-place' || key == 'publisher-place') { - return true; - } - - // Skip for now, since we don't support date ranges - // https://github.com/zotero/zotero/issues/3030 - if (key == 'issued') { - return true; - } - // Fields let possibleFields = fieldNames.get(key); + // No valid fields if (possibleFields) { let added = false; diff --git a/test/tests/schemaTest.js b/test/tests/schemaTest.js index c370473577..4469add17d 100644 --- a/test/tests/schemaTest.js +++ b/test/tests/schemaTest.js @@ -236,6 +236,20 @@ describe("Zotero.Schema", function() { assert.equal(item.getField('extra'), 'type: invalid'); assert.isTrue(item.synced); }); + + it("shouldn't migrate certain fields temporarily", async function () { + var item = await createDataObject('item', { itemType: 'book' }); + var extra = 'event-place: Event Place\npublisher-place: Publisher Place\nIssued: 2020/2023'; + item.setField('extra', extra); + item.synced = true; + await item.saveTx(); + + await migrate(); + + assert.equal(item.getField('place'), ''); + assert.equal(item.getField('date'), ''); + assert.equal(item.getField('extra'), extra); + }); }); }); diff --git a/test/tests/utilities_internalTest.js b/test/tests/utilities_internalTest.js index 79dccb20bf..30d08cc701 100644 --- a/test/tests/utilities_internalTest.js +++ b/test/tests/utilities_internalTest.js @@ -248,7 +248,7 @@ describe("Zotero.Utilities.Internal", function () { assert.equal(creators[0].name, 'Bar'); }); - it.skip("should extract a CSL date field", function () { + it("should extract a CSL date field", function () { var str = 'issued: 2000'; var { fields, extra } = Zotero.Utilities.Internal.extractExtraFields(str); assert.equal(fields.size, 1); @@ -286,13 +286,11 @@ describe("Zotero.Utilities.Internal", function () { }); it("should extract the citeproc-js cheater syntax", function () { - //var issued = '{:number-of-pages:11}\n{:issued:2014}'; - var issued = '{:number-of-pages:11}\n{:archive_location:Location}'; + var issued = '{:number-of-pages:11}\n{:issued:2014}'; var { fields, extra } = Zotero.Utilities.Internal.extractExtraFields(issued); assert.equal(fields.size, 2); assert.equal(fields.get('numPages'), 11); - assert.equal(fields.get('archiveLocation'), 'Location'); - //assert.equal(fields.get('date'), 2014); + assert.equal(fields.get('date'), 2014); assert.strictEqual(extra, ''); }); @@ -302,23 +300,6 @@ describe("Zotero.Utilities.Internal", function () { assert.equal(fields.size, 0); assert.strictEqual(extra, str); }); - - it("should ignore both Event Place and Publisher Place (temporary)", function () { - var str = "Event Place: Foo\nPublisher Place: Bar"; - var { fields, extra } = Zotero.Utilities.Internal.extractExtraFields(str); - Zotero.debug([...fields.entries()]); - assert.equal(fields.size, 0); - assert.equal(extra, "Event Place: Foo\nPublisher Place: Bar"); - }); - - // Re-enable "should extract a CSL date field" and cheater-syntax lines above when removed - it("should ignore Issued (temporary)", function () { - var str = "Issued: 1994/1998"; - var { fields, extra } = Zotero.Utilities.Internal.extractExtraFields(str); - Zotero.debug([...fields.entries()]); - assert.equal(fields.size, 0); - assert.equal(extra, "Issued: 1994/1998"); - }); }); describe("#combineExtraFields", function () {