From 02a35445903dad78afbb0f198d6afec8b1e6544f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 19 May 2008 15:16:25 -0700 Subject: [PATCH] Search result now return notes from multiple notebooks. - change model.Notebook.sql_search_notes() to be a static method - take a first_notebook_id argument and a user_id argument - join user_notebook on notebook_id and matches on user_notebook.user_id instead of notebook_id - order by notebook_id = first_notebook desc, rank instead of just rank - include search result for notebooks readable by anonymous user, but only if such a notebook is given as first_notebook_id - update code that calls model.Notebook.sql_search_notes() - update model.Note.to_dict() to include notebook_id - modify Wiki.display_search_results() - separate search results for the current notebook and results for all other notebooks - indicate which notebook each result is from - if there are results in other notebooks but no results in the current notebook, indicate that clearly - when you click on a result note link in the current notebook, it should simply open a new note - when you click on a result note link in another notebook, it should open in a new window - test in IE - update unit tests for: - controller.Notebooks.search() - model.Notebook.sql_search_notes() - model.Note.to_dict() --- controller/Notebooks.py | 17 ++++++++-------- controller/test/Test_controller.py | 19 +++++++++++------- model/Note.py | 1 + model/Notebook.py | 24 +++++++++++++--------- model/test/Test_note.py | 1 + static/css/note.css | 7 +++++++ static/js/Editor.js | 3 +++ static/js/Wiki.js | 32 ++++++++++++++++++++++++------ 8 files changed, 73 insertions(+), 31 deletions(-) diff --git a/controller/Notebooks.py b/controller/Notebooks.py index 00a0444..1ef325a 100644 --- a/controller/Notebooks.py +++ b/controller/Notebooks.py @@ -900,13 +900,13 @@ class Notebooks( object ): ) def search( self, notebook_id, search_text, user_id ): """ - Search the notes within a particular notebook for the given search text. Note that the search - is case-insensitive, and all HTML tags are ignored. Notes with title matches are generally - ranked higher than matches that are only in the note contents. The returned notes include - content summaries with the search terms highlighted. + Search the notes within all notebooks that the user has access to for the given search text. + Note that the search is case-insensitive, and all HTML tags are ignored. Notes with title + matches are generally ranked higher than matches that are only in the note contents. The + returned notes include content summaries with the search terms highlighted. @type notebook_id: unicode - @param notebook_id: id of notebook to search + @param notebook_id: id of notebook to show first in search results @type search_text: unicode @param search_text: search term @type user_id: unicode or NoneType @@ -920,9 +920,8 @@ class Notebooks( object ): if not self.__users.check_access( user_id, notebook_id ): raise Access_error() - notebook = self.__database.load( Notebook, notebook_id ) - - if not notebook: + anonymous = self.__database.select_one( User, User.sql_load_by_username( u"anonymous" ), use_cache = True ) + if not anonymous: raise Access_error() MAX_SEARCH_TEXT_LENGTH = 256 @@ -932,7 +931,7 @@ class Notebooks( object ): if len( search_text ) == 0: raise Validation_error( u"search_text", None, unicode, message = u"is missing" ) - notes = self.__database.select_many( Note, notebook.sql_search_notes( search_text ) ) + notes = self.__database.select_many( Note, Notebook.sql_search_notes( user_id, anonymous.object_id, notebook_id, search_text ) ) return dict( notes = notes, diff --git a/controller/test/Test_controller.py b/controller/test/Test_controller.py index 585186b..20cf8f7 100644 --- a/controller/test/Test_controller.py +++ b/controller/test/Test_controller.py @@ -276,20 +276,25 @@ class Test_controller( object ): Notebook.sql_load_note_by_title = lambda self, title: \ lambda database: sql_load_note_by_title( self, title, database ) - def sql_search_notes( self, search_text, database ): - notes = [] + def sql_search_notes( user_id, anonymous_user_id, first_notebook_id, search_text, database ): + first_notes = [] + other_notes = [] search_text = search_text.lower() for ( object_id, obj_list ) in database.objects.items(): obj = obj_list[ -1 ] - if isinstance( obj, Note ) and obj.notebook_id == self.object_id and \ + if isinstance( obj, Note ) and ( database.user_notebook.get( user_id ) or \ + ( database.user_notebook.get( anonymous_user_id ) and note.notebook_id == first_notebook_id ) ) and \ search_text in obj.contents.lower(): - notes.append( obj ) + if obj.notebook_id == first_notebook_id: + first_notes.append( obj ) + else: + other_notes.append( obj ) - return notes + return first_notes + other_notes - Notebook.sql_search_notes = lambda self, search_text: \ - lambda database: sql_search_notes( self, search_text, database ) + Notebook.sql_search_notes = staticmethod( lambda user_id, anonymous_user_id, first_notebook_id, search_text: \ + lambda database: sql_search_notes( user_id, anonymous_user_id, first_notebook_id, search_text, database ) ) def sql_highest_note_rank( self, database ): max_rank = -1 diff --git a/model/Note.py b/model/Note.py index 44f9a38..7088b6e 100644 --- a/model/Note.py +++ b/model/Note.py @@ -171,6 +171,7 @@ class Note( Persistent ): d.update( dict( contents = self.__contents, summary = self.__summary, + notebook_id = self.__notebook_id, title = self.__title, deleted_from_id = self.__deleted_from_id, user_id = self.__user_id, diff --git a/model/Notebook.py b/model/Notebook.py index 8323e68..c8611d4 100644 --- a/model/Notebook.py +++ b/model/Notebook.py @@ -166,32 +166,38 @@ class Notebook( Persistent ): """ return "select id, revision, title, contents, notebook_id, startup, deleted_from_id, rank, user_id from note_current where notebook_id = %s and title = %s;" % ( quote( self.object_id ), quote( title ) ) - def sql_search_notes( self, search_text ): + @staticmethod + def sql_search_notes( user_id, anonymous_user_id, first_notebook_id, search_text ): """ - Return a SQL string to perform a full-text search for notes whose contents contain the given - search_text. This is a case-insensitive search. + Return a SQL string to perform a full-text search for notes within notebooks readable by the + given user whose contents contain the given search_text. This is a case-insensitive search. @type search_text: unicode @param search_text: text to search for within the notes """ # strip out all search operators - search_text = self.SEARCH_OPERATORS.sub( u"", search_text ).strip() + search_text = Notebook.SEARCH_OPERATORS.sub( u"", search_text ).strip() # join all words with boolean "and" operator - search_text = u"&".join( self.WHITESPACE_PATTERN.split( search_text ) ) + search_text = u"&".join( Notebook.WHITESPACE_PATTERN.split( search_text ) ) return \ """ select id, revision, title, contents, notebook_id, startup, deleted_from_id, rank, user_id, null, headline( drop_html_tags( contents ), query ) as summary from ( select - id, revision, title, contents, notebook_id, startup, deleted_from_id, rank_cd( search, query ) as rank, user_id, null, query + note_current.id, note_current.revision, note_current.title, note_current.contents, + note_current.notebook_id, note_current.startup, note_current.deleted_from_id, + rank_cd( search, query ) as rank, note_current.user_id, null, query from - note_current, to_tsquery( 'default', %s ) query + note_current, user_notebook, to_tsquery( 'default', %s ) query where - notebook_id = %s and query @@ search order by rank desc limit 20 + note_current.notebook_id = user_notebook.notebook_id and ( user_notebook.user_id = %s or + ( user_notebook.user_id = %s and note_current.notebook_id = %s ) ) and + query @@ search order by note_current.notebook_id = %s desc, rank desc limit 20 ) as sub; - """ % ( quote( search_text ), quote( self.object_id ) ) + """ % ( quote( search_text ), quote( user_id ), quote( anonymous_user_id ), + quote( first_notebook_id ), quote( first_notebook_id ) ) def sql_highest_note_rank( self ): """ diff --git a/model/test/Test_note.py b/model/test/Test_note.py index bc0f901..3a466cb 100644 --- a/model/test/Test_note.py +++ b/model/test/Test_note.py @@ -165,6 +165,7 @@ class Test_note( object ): assert datetime.now( tz = utc ) - d.get( "revision" ) < self.delta assert d.get( "contents" ) == self.contents assert d.get( "summary" ) == self.summary + assert d.get( "notebook_id" ) == self.notebook_id assert d.get( "title" ) == self.title assert d.get( "deleted_from_id" ) == None assert d.get( "user_id" ) == self.user_id diff --git a/static/css/note.css b/static/css/note.css index 0a2d931..424f8e3 100644 --- a/static/css/note.css +++ b/static/css/note.css @@ -40,6 +40,13 @@ del a { color: red; } +hr { + border: 0; + color: #000000; + background-color: #000000; + height: 1px; +} + .button { border-style: outset; border-width: 0px; diff --git a/static/js/Editor.js b/static/js/Editor.js index b9a3f25..bb911a1 100644 --- a/static/js/Editor.js +++ b/static/js/Editor.js @@ -377,6 +377,8 @@ Editor.prototype.mouse_clicked = function ( event ) { // search through the tree of elements containing the clicked target. if a link isn't found, bail var link = event.target() + if ( !link ) false; + while ( link.nodeName != "A" ) { link = link.parentNode; if ( !link ) @@ -539,6 +541,7 @@ Editor.prototype.find_link_at_cursor = function () { if ( this.iframe.contentWindow && this.iframe.contentWindow.getSelection ) { // browsers such as Firefox var selection = this.iframe.contentWindow.getSelection(); var link = selection.anchorNode; + if ( !link ) return null; while ( link.nodeName != "A" ) { link = link.parentNode; diff --git a/static/js/Wiki.js b/static/js/Wiki.js index f681bc7..becfedb 100644 --- a/static/js/Wiki.js +++ b/static/js/Wiki.js @@ -35,11 +35,12 @@ function Wiki( invoker ) { // grab the current notebook from the list of available notebooks this.notebooks = evalJSON( getElement( "notebooks" ).value ); + this.notebooks_by_id = {}; + for ( var i in this.notebooks ) { - if ( this.notebooks[ i ].object_id == this.notebook_id ) { + this.notebooks_by_id[ this.notebooks[ i ].object_id ] = this.notebooks[ i ]; + if ( this.notebooks[ i ].object_id == this.notebook_id ) this.notebook = this.notebooks[ i ] - break; - } } if ( this.notebook && this.notebook.read_write ) { @@ -1473,12 +1474,13 @@ Wiki.prototype.display_search_results = function ( result ) { return; } - // otherwise, there are multiple search results, so create a "magic" search results note. but - // first close any open search results notes + // create a "magic" search results note. but first close any open search results notes if ( this.search_results_editor ) this.search_results_editor.shutdown(); var list = createDOM( "span", {} ); + var other_notebooks_section = false; + for ( var i in result.notes ) { var note = result.notes[ i ] if ( !note.title ) continue; @@ -1497,9 +1499,27 @@ Wiki.prototype.display_search_results = function ( result ) { summary_span.setAttribute( "class", "search_results_summary" ); summary_span.innerHTML = summary; + // when a link is clicked for a note from a notebook other than the current one, open it in a + // new window + var link_attributes = { "href": "/notebooks/" + note.notebook_id + "?note_id=" + note.object_id }; + if ( note.notebook_id != this.notebook_id ) { + link_attributes[ "target" ] = "_new"; + + if ( !other_notebooks_section ) { + other_notebooks_section = true; + if ( i == 0 ) + appendChildNodes( list, createDOM( "p", {}, "No matching notes in this notebook." ) ); + + appendChildNodes( list, createDOM( "hr" ), createDOM( "h4", {}, "other notebooks" ) ); + } + } + + var notebook = this.notebooks_by_id[ note.notebook_id ]; + appendChildNodes( list, createDOM( "p", {}, - createDOM( "a", { "href": "/notebooks/" + this.notebook_id + "?note_id=" + note.object_id }, note.title ), + createDOM( "a", link_attributes, note.title ), + other_notebooks_section && notebook && createDOM( "span", { "class": "small_text" }, " (", notebook.name, ")" ) || null, createDOM( "br" ), summary_span )