From d0d3f80a612b5553db46c1f6d15388b3c1f99a76 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 6 Nov 2006 12:05:57 +0000 Subject: [PATCH] Much improved date handling in searches and autocomplete Fixes #338, Dates in search window should be converted to UTC before searching Addresses #220, Add a more friendly way to enter dates in search window and accessDate field - Unless passed a full datetime, the search system automatically parses the string with strToDate(), so one can search for an accessDate, etc., using a freeform phrase. It will use the SQL parts where available and also search for remaining words individually, so "November 6, 2006" will find '2006-10-06 Nov. 6 2006' and "Summer 2006" will find '2006-00-00 Summer 2006". It will also properly handle SQL parts, so "August 2006" in the is/isNot operator will use "LIKE '2006-08-__'" (underscore being the single-character wildcard) and isBefore will use '2006-08-00' - Stored UTC dates are converted to localtime in the search system when searching on just a date part, since otherwise searching for a local date with a UTC timestamp after midnight would be unsuccessful - Date field autocomplete is now disabled in the search dialog, but on the off-chance that it's used somewhere, the autocomplete will now return just the user part of a multipart date field - Access date autocomplete only returns date, not datetime - Fix for Date Added and Date Modified showing as UTC - Date.isSQLDateTime() Known issues: - accessDate field in metadata pane still requires SQL format - Proper parsing of search dates with no years (e.g. searching for "August 25") depends on #389, "Date.strToDate() should return available parts even if no year" --- .../content/zotero/bindings/searchtextbox.xml | 1 + .../content/zotero/bindings/zoterosearch.xml | 21 ++ chrome/content/zotero/itemPane.js | 60 +++--- chrome/content/zotero/xpcom/search.js | 204 ++++++++++++++---- chrome/content/zotero/xpcom/zotero.js | 7 + components/zotero-autocomplete.js | 30 ++- 6 files changed, 246 insertions(+), 77 deletions(-) diff --git a/chrome/content/zotero/bindings/searchtextbox.xml b/chrome/content/zotero/bindings/searchtextbox.xml index 2bd94654a..86d22ec25 100644 --- a/chrome/content/zotero/bindings/searchtextbox.xml +++ b/chrome/content/zotero/bindings/searchtextbox.xml @@ -116,6 +116,7 @@ switch (condition) { // Skip autocomplete for these fields + case 'date': case 'note': case 'extra': break; diff --git a/chrome/content/zotero/bindings/zoterosearch.xml b/chrome/content/zotero/bindings/zoterosearch.xml index ee8fc61fc..8509171c5 100644 --- a/chrome/content/zotero/bindings/zoterosearch.xml +++ b/chrome/content/zotero/bindings/zoterosearch.xml @@ -394,6 +394,16 @@ this.id('conditionsmenu').value = condition['condition']; } + // Convert datetimes from UTC to localtime + if ((condition['condition']=='accessDate' || + condition['condition']=='dateAdded' || + condition['condition']=='dateModified') && + Zotero.Date.isSQLDateTime(condition['value'])){ + + condition['value'] = + Zotero.Date.dateToSQL(Zotero.Date.sqlToDate(condition['value'], true)); + } + this.mode = condition['mode']; this.id('operatorsmenu').value = condition['operator']; this.value = prefix + condition['value']; @@ -416,12 +426,23 @@ if (!this.id('valuefield').hidden) { var value = this.id('valuefield').value; + + // Convert datetimes to UTC before saving + if ((this.id('conditionsmenu').value=='accessDate' || + this.id('conditionsmenu').value=='dateAdded' || + this.id('conditionsmenu').value=='dateModified') && + Zotero.Date.isSQLDateTime(value)){ + + var value = Zotero.Date.dateToSQL(Zotero.Date.sqlToDate(value), true); + } + // Append mode to condition if (this.id('valuefield').mode){ condition += '/' + this.id('valuefield').mode; } } + // isInTheLast operator else if (!this.id('value-date-age').hidden) { var value = this.id('value-date-age').value; diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js index 7e4806d8b..21ea3362c 100644 --- a/chrome/content/zotero/itemPane.js +++ b/chrome/content/zotero/itemPane.js @@ -260,7 +260,7 @@ var ZoteroItemPane = new function() } var valueElement = createValueElement( - val, editable ? fieldNames[i] : null, tabindex + val, fieldNames[i], tabindex, !editable ); var label = document.createElement("label"); @@ -733,7 +733,7 @@ var ZoteroItemPane = new function() "ZoteroItemPane.disableButton(this); ZoteroItemPane.addCreatorRow('', '', " + (creatorTypeID ? creatorTypeID : 'false') + ", " + fieldMode + ", true);"); } - function createValueElement(valueText, fieldName, tabindex) + function createValueElement(valueText, fieldName, tabindex, noedit) { if (fieldName=='extra') { @@ -744,41 +744,39 @@ var ZoteroItemPane = new function() var valueElement = document.createElement("label"); } - if(fieldName) - { + valueElement.setAttribute('fieldname',fieldName); + + if (!noedit){ valueElement.setAttribute('flex', 1); - valueElement.setAttribute('fieldname',fieldName); valueElement.setAttribute('tabindex', tabindex); valueElement.setAttribute('onclick', 'ZoteroItemPane.showEditor(this)'); valueElement.className = 'zotero-clicky'; + } + + switch (fieldName){ + case 'tag': + _tabIndexMaxTagsFields = Math.max(_tabIndexMaxTagsFields, tabindex); + break; - switch (fieldName) - { - case 'tag': - _tabIndexMaxTagsFields = Math.max(_tabIndexMaxTagsFields, tabindex); - break; - - // Display the SQL date as a tooltip for the date field - case 'date': - valueElement.setAttribute('tooltiptext', - Zotero.Date.multipartToSQL(_itemBeingEdited.getField('date', true))); - break; - - // Convert dates from UTC - case 'dateAdded': - case 'dateModified': - case 'accessDate': - if (valueText) - { - var date = Zotero.Date.sqlToDate(valueText, true); - valueText = date ? date.toLocaleString() : ''; - } - break; - } + // Display the SQL date as a tooltip for the date field + case 'date': + valueElement.setAttribute('tooltiptext', + Zotero.Date.multipartToSQL(_itemBeingEdited.getField('date', true))); + break; - if (fieldName.indexOf('firstName')!=-1){ - valueElement.setAttribute('flex', '1'); - } + // Convert dates from UTC + case 'dateAdded': + case 'dateModified': + case 'accessDate': + if (valueText){ + var date = Zotero.Date.sqlToDate(valueText, true); + valueText = date ? date.toLocaleString() : ''; + } + break; + } + + if (fieldName.indexOf('firstName')!=-1){ + valueElement.setAttribute('flex', '1'); } var firstSpace; diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js index 4d3f8631c..6eb92f136 100644 --- a/chrome/content/zotero/xpcom/search.js +++ b/chrome/content/zotero/xpcom/search.js @@ -330,6 +330,8 @@ Zotero.Search.prototype.getSQLParams = function(){ * Build the SQL query for the search */ Zotero.Search.prototype._buildQuery = function(){ + var utils = new Zotero.Utilities(); + var sql = 'SELECT itemID FROM items'; var sqlParams = []; // Separate ANY conditions for 'required' condition support @@ -516,51 +518,173 @@ Zotero.Search.prototype._buildQuery = function(){ } if (!skipOperators){ - condSQL += condition['field']; - switch (condition['operator']){ - case 'contains': - case 'doesNotContain': // excluded with NOT IN above - condSQL += ' LIKE ?'; - condSQLParams.push('%' + condition['value'] + '%'); - break; - - case 'is': - case 'isNot': // excluded with NOT IN above - condSQL += '=?'; - condSQLParams.push(condition['value']); - break; + // Special handling for date fields + // + // Note: We assume full datetimes are already UTC and don't + // need to be handle specially + if ((condition['name']=='dateAdded' || + condition['name']=='dateModified' || + condition['name']=='datefield') && + !Zotero.Date.isSQLDateTime(condition['value'])){ - /* - case 'beginsWith': - condSQL += '=?'; - condSQLParams.push(condition['value'] + '%'); - break; - */ + switch (condition['operator']){ + case 'is': + case 'isNot': + var parseDate = true; + var alt = '__'; + var useFreeform = true; + break; + + case 'isBefore': + var parseDate = true; + var alt = '00'; + var useFreeform = false; + break; + + case 'isAfter': + var parseDate = true; + // '__' used here just so the > string comparison + // doesn't match dates in the specified year + var alt = '__'; + var useFreeform = false; + break; + + case 'isInTheLast': + var parseDate = false; + break; + + default: + throw ('Invalid date field operator in search'); + } - case 'isLessThan': - condSQL += '?'; - condSQLParams.push({int:condition['value']}); - break; + // Search on SQL date -- underscore is + // single-character wildcard + // + // If isBefore or isAfter, month and day fall back + // to '00' so that a search for just a year works + // (and no year will just not find anything) + var sqldate = dateparts['year'] ? + utils.lpad(dateparts['year'], '0', 4) : '____'; + sqldate += '-' + sqldate += dateparts['month'] ? + utils.lpad(dateparts['month'] + 1, '0', 2) : alt; + sqldate += '-'; + sqldate += dateparts['day'] ? + utils.lpad(dateparts['day'], '0', 2) : alt; - case 'isBefore': - condSQL += ''0000-00-00'"; + break; + + case 'isAfter': + condSQL += '>?'; + break; + } + + condSQLParams.push({string:sqldate}); + } - case 'isAfter': - condSQL += '>?'; - condSQLParams.push({string:condition['value']}); - break; + // Search for any remaining parts individually + if (useFreeform && dateparts['part']){ + go = true; + var parts = dateparts['part'].split(' '); + for each (var part in parts){ + condSQL += " AND SUBSTR(" + condition['field'] + ", 12, 100)"; + condSQL += " LIKE ?"; + condSQLParams.push('%' + part + '%'); + } + } - case 'isInTheLast': - condSQL += ">DATE('NOW', ?)"; - condSQLParams.push({string: '-' + condition['value']}); - break; + // If neither part used, invalidate clause + if (!go){ + condSQL += '=0'; + } + } + + else { + switch (condition['operator']){ + case 'isInTheLast': + condSQL += ">DATE('NOW', 'localtime', ?)"; // e.g. ('NOW', '-10 DAYS') + condSQLParams.push({string: '-' + condition['value']}); + break; + } + } + } + + // Non-date fields + else { + condSQL += condition['field']; + switch (condition['operator']){ + case 'contains': + case 'doesNotContain': // excluded with NOT IN above + condSQL += ' LIKE ?'; + condSQLParams.push('%' + condition['value'] + '%'); + break; + + case 'is': + case 'isNot': // excluded with NOT IN above + condSQL += '=?'; + condSQLParams.push(condition['value']); + break; + + /* + case 'beginsWith': + condSQL += '=?'; + condSQLParams.push(condition['value'] + '%'); + break; + */ + + case 'isLessThan': + condSQL += '?'; + condSQLParams.push({int:condition['value']}); + break; + + // Next two only used with full datetimes + case 'isBefore': + condSQL += '?'; + condSQLParams.push({string:condition['value']}); + break; + } } } @@ -877,7 +1001,7 @@ Zotero.SearchConditions = new function(){ isInTheLast: true }, table: 'itemData', - field: 'SUBSTR(value, 1, 10)', // use only beginning of multipart dates + field: 'value', aliases: ['accessDate', 'date'], template: true // mark for special handling }, diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index fcd2c4414..aab6c51d8 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -684,6 +684,7 @@ Zotero.Date = new function(){ this.multipartToSQL = multipartToSQL; this.multipartToStr = multipartToStr; this.isSQLDate = isSQLDate; + this.isSQLDateTime = isSQLDateTime; this.sqlHasYear = sqlHasYear; this.sqlHasMonth = sqlHasMonth; this.sqlHasDay = sqlHasDay; @@ -963,6 +964,7 @@ Zotero.Date = new function(){ // Regexes for multipart and SQL dates var _multipartRE = /^[0-9]{4}\-[0-9]{2}\-[0-9]{2} /; var _sqldateRE = /^[0-9]{4}\-[0-9]{2}\-[0-9]{2}/; + var _sqldatetimeRE = /^[0-9]{4}\-[0-9]{2}\-[0-9]{2} ([0-1][0-9]|[2][0-3]):([0-5][0-9]):([0-5][0-9])/; /** * Tests if a string is a multipart date string @@ -1012,6 +1014,11 @@ Zotero.Date = new function(){ } + function isSQLDateTime(str){ + return _sqldatetimeRE.test(str); + } + + function sqlHasYear(sqldate){ return isSQLDate(sqldate) && sqldate.substr(0,4)!='0000'; } diff --git a/components/zotero-autocomplete.js b/components/zotero-autocomplete.js index 9d6a2ba3f..e7163dbe5 100644 --- a/components/zotero-autocomplete.js +++ b/components/zotero-autocomplete.js @@ -209,15 +209,26 @@ ZoteroAutoComplete.prototype.startSearch = function(searchString, searchParam, break; case 'title': - // DEBUG: These two probably won't be necesary once there's a better - // date entry method - case 'dateModified': - case 'dateAdded': var sql = "SELECT DISTINCT " + searchParam + " FROM items " + "WHERE " + searchParam + " LIKE ? ORDER BY " + searchParam; var results = this._zotero.DB.columnQuery(sql, searchString + '%'); break; + case 'dateModified': + case 'dateAdded': + var sql = "SELECT DISTINCT DATE(" + searchParam + ", 'localtime') FROM items " + + "WHERE " + searchParam + " LIKE ? ORDER BY " + searchParam; + var results = this._zotero.DB.columnQuery(sql, searchString + '%'); + break; + + case 'accessDate': + var fieldID = this._zotero.ItemFields.getID('accessDate'); + + var sql = "SELECT DISTINCT DATE(value, 'localtime') FROM itemData " + + "WHERE fieldID=? AND value LIKE ? ORDER BY value"; + var results = this._zotero.DB.columnQuery(sql, [fieldID, searchString + '%']); + break; + default: var sql = "SELECT fieldID FROM fields WHERE fieldName=?"; var fieldID = this._zotero.DB.valueQuery(sql, {string:searchParam}); @@ -229,8 +240,15 @@ ZoteroAutoComplete.prototype.startSearch = function(searchString, searchParam, break; } - var sql = "SELECT DISTINCT value FROM itemData WHERE fieldID=?1 AND " - + "value LIKE ?2 " + // We don't use date autocomplete anywhere, but if we're not + // disallowing it altogether, we should at least do it right and + // use the user part of the multipart field + var valueField = searchParam=='date' ? 'SUBSTR(value, 12, 100)' : 'value'; + + var sql = "SELECT DISTINCT " + valueField; + sql += " FROM itemData WHERE fieldID=?1 AND " + valueField; + sql += " LIKE ?2 " + var sqlParams = [fieldID, searchString + '%']; if (extra){ sql += "AND value NOT IN (SELECT value FROM itemData "