From b4a40d2c251d07af0542b0c104d82b28f29d40ed Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 30 Oct 2008 15:26:27 -0700 Subject: [PATCH] More work on the discussion forums. --- NEWS | 1 + controller/Forums.py | 57 +++++++++++++++--- controller/Notebooks.py | 52 ++++++++++++++-- controller/test/Test_notebooks.py | 2 +- model/Notebook.py | 14 ++++- static/js/Wiki.js | 99 ++++++++++++++++++++++--------- view/Main_page.py | 65 ++++++++++++++++---- 7 files changed, 231 insertions(+), 59 deletions(-) diff --git a/NEWS b/NEWS index 2f54333..dc1a606 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ * Laid some of the foundational groundwork for future tags support. * Made the subscription pricing page a little less confusing by hiding some of the bigger plans by default. + * Increased the limit on characters per note from 25,000 to 50,000. * Fixed an occasional bug that caused unexpected logouts. The solution was to move the session information into the database where it could be properly locked. diff --git a/controller/Forums.py b/controller/Forums.py index 9377c11..1188905 100644 --- a/controller/Forums.py +++ b/controller/Forums.py @@ -1,9 +1,10 @@ import cherrypy from model.User import User from model.Notebook import Notebook +from model.Note import Note from model.Tag import Tag from Expose import expose -from Validate import validate, Valid_string +from Validate import validate, Valid_string, Valid_int from Database import Valid_id, end_transaction from Users import grab_user_id from Notebooks import Notebooks @@ -64,6 +65,8 @@ class Forums( object ): class Forum( object ): + DEFAULT_THREAD_NAME = u"new discussion" + def __init__( self, database, notebooks, users, name ): """ Create a new Forum object, representing a single forum. @@ -108,12 +111,13 @@ class Forum( object ): if anonymous is None: raise Access_error() - threads = self.__database.select_many( + # load a list of the threads in this forum, excluding those with a default name + threads = [ thread for thread in self.__database.select_many( Notebook, anonymous.sql_load_notebooks( parents_only = False, undeleted_only = True, tag_name = u"forum", tag_value = self.__name ) - ) + ) if thread.name != self.DEFAULT_THREAD_NAME ] # put threads in reverse chronological order by creation date threads.reverse() @@ -126,9 +130,40 @@ class Forum( object ): result[ "threads" ] = threads return result - # default() is just an alias for Notebooks.default() - def default( self, *args, **kwargs ): - return self.__notebooks.default( *args, **kwargs ) + @expose( view = Main_page ) + @end_transaction + @grab_user_id + @validate( + thread_id = Valid_id(), + start = Valid_int( min = 0 ), + count = Valid_int( min = 1, max = 50 ), + note_id = Valid_id( none_okay = True ), + user_id = Valid_id( none_okay = True ), + ) + def default( self, thread_id, start = 0, count = 10, note_id = None, user_id = None ): + """ + Provide the information necessary to display a forum thread. + + @type thread_id: unicode + @param thread_id: id of thread notebook to display + @type start: unicode or NoneType + @param start: index of recent note to start with (defaults to 0, the most recent note) + @type count: int or NoneType + @param count: number of recent notes to display (defaults to 10 notes) + @type note_id: unicode or NoneType + @param note_id: id of single note to load (optional) + @rtype: unicode + @return: rendered HTML page + @raise Validation_error: one of the arguments is invalid + """ + result = self.__users.current( user_id ) + result.update( self.__notebooks.old_notes( thread_id, start, count, user_id ) ) + + # if a single note was requested, just return that one note + if note_id: + result[ "notes" ] = [ note for note in result[ "notes" ] if note.object_id == note_id ] + + return result default.exposed = True @@ -140,7 +175,8 @@ class Forum( object ): ) def create_thread( self, user_id ): """ - Create a new forum post and give it a default name. Then redirect to that new post thread. + Create a new forum thread with a blank post, and give the thread a default name. Then redirect + to that new thread. @type user_id: unicode or NoneType @param user_id: id of current logged-in user (if any) @@ -162,7 +198,7 @@ class Forum( object ): # create the new notebook thread thread_id = self.__database.next_id( Notebook, commit = False ) - thread = Notebook.create( thread_id, u"new forum post", user_id = user.object_id ) + thread = Notebook.create( thread_id, self.DEFAULT_THREAD_NAME, user_id = user.object_id ) self.__database.save( thread, commit = False ) # associate the forum tag with the new notebook thread @@ -178,6 +214,11 @@ class Forum( object ): commit = False, ) + # create a blank post in which the user can start off the thread + note_id = self.__database.next_id( Notebook, commit = False ) + note = Note.create( note_id, u"

", notebook_id = thread_id, startup = True, rank = 0, user_id = user_id ) + self.__database.save( note, commit = False ) + self.__database.commit() return dict( diff --git a/controller/Notebooks.py b/controller/Notebooks.py index 1b932bb..7f6ed27 100644 --- a/controller/Notebooks.py +++ b/controller/Notebooks.py @@ -656,7 +656,7 @@ class Notebooks( object ): @validate( notebook_id = Valid_id(), note_id = Valid_id(), - contents = Valid_string( min = 1, max = 25000, escape_html = False ), + contents = Valid_string( min = 1, max = 50000, escape_html = False ), startup = Valid_bool(), previous_revision = Valid_revision( none_okay = True ), user_id = Valid_id( none_okay = True ), @@ -685,7 +685,8 @@ class Notebooks( object ): @return: { 'new_revision': User_revision of saved note, or None if nothing was saved 'previous_revision': User_revision immediately before new_revision, or None if the note is new - 'storage_bytes': current storage usage by user, + 'storage_bytes': current storage usage by user + 'rank': integer rank of the saved note, or None } @raise Access_error: the current user doesn't have access to the given notebook @raise Validation_error: one of the arguments is invalid @@ -768,6 +769,7 @@ class Notebooks( object ): new_revision = new_revision, previous_revision = previous_revision, storage_bytes = user and user.storage_bytes or 0, + rank = note.rank, ) @expose( view = Json ) @@ -1332,6 +1334,13 @@ class Notebooks( object ): @raise Validation_error: one of the arguments is invalid """ notebook = self.__users.load_notebook( user_id, notebook_id, read_write = True, owner = True ) + + # special case to allow the creator of a READ_WRITE_FOR_OWN_NOTES notebook to rename it + if notebook is None: + notebook = self.__users.load_notebook( user_id, notebook_id, read_write = True ) + if not ( notebook.read_write == Notebook.READ_WRITE_FOR_OWN_NOTES and notebook.user_id == user_id ): + raise Access_error() + user = self.__database.load( User, user_id ) if not user or not notebook: @@ -1670,7 +1679,7 @@ class Notebooks( object ): def recent_notes( self, notebook_id, start = 0, count = 10, user_id = None ): """ - Return the given notebook's recently updated notes in reverse chronological order by creation + Return the given notebook's recently created notes in reverse chronological order by creation time. @type notebook_id: unicode @@ -1690,11 +1699,42 @@ class Notebooks( object ): if notebook is None: raise Access_error() - recent_notes = self.__database.select_many( Note, notebook.sql_load_recent_notes( start, count ) ) + notes = self.__database.select_many( Note, notebook.sql_load_recent_notes( start, count ) ) result = self.__users.current( user_id ) result.update( self.contents( notebook_id, user_id = user_id ) ) - result[ "notes" ] = recent_notes + result[ "notes" ] = notes + result[ "start" ] = start + result[ "count" ] = count + + return result + + def old_notes( self, notebook_id, start = 0, count = 10, user_id = None ): + """ + Return the given notebook's oldest notes in chronological order by creation time. + + @type notebook_id: unicode + @param notebook_id: id of the notebook containing the notes + @type start: unicode or NoneType + @param start: index of recent note to start with (defaults to 0, the oldest note) + @type count: int or NoneType + @param count: number of notes to return (defaults to 10 notes) + @type user_id: unicode or NoneType + @param user_id: id of current logged-in user (if any) + @rtype: dict + @return: data for Main_page() constructor + @raise Access_error: the current user doesn't have access to the given notebook or note + """ + notebook = self.__users.load_notebook( user_id, notebook_id ) + + if notebook is None: + raise Access_error() + + notes = self.__database.select_many( Note, notebook.sql_load_recent_notes( start, count, reverse = True ) ) + + result = self.__users.current( user_id ) + result.update( self.contents( notebook_id, user_id = user_id ) ) + result[ "notes" ] = notes result[ "start" ] = start result[ "count" ] = count @@ -1813,7 +1853,7 @@ class Notebooks( object ): else: break - contents = Valid_string( max = 25000, escape_html = plaintext, require_link_target = True )( row[ content_column ] ) + contents = Valid_string( max = 50000, escape_html = plaintext, require_link_target = True )( row[ content_column ] ) if plaintext: contents = contents.replace( u"\n", u"
" ) diff --git a/controller/test/Test_notebooks.py b/controller/test/Test_notebooks.py index 2f51e79..bb5161f 100644 --- a/controller/test/Test_notebooks.py +++ b/controller/test/Test_notebooks.py @@ -1912,7 +1912,7 @@ class Test_notebooks( Test_controller ): # save over an existing note supplying new (too long) contents and a new title previous_revision = self.note.revision - new_note_contents = u"

new title

new blah" * 962 + new_note_contents = u"

new title

new blah" * 1923 result = self.http_post( "/notebooks/save_note/", dict( notebook_id = self.notebook.object_id, note_id = self.note.object_id, diff --git a/model/Notebook.py b/model/Notebook.py index d4a24cc..02c1ed5 100644 --- a/model/Notebook.py +++ b/model/Notebook.py @@ -151,7 +151,7 @@ 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 startup = 't' order by rank;" % quote( self.object_id ) - def sql_load_recent_notes( self, start = 0, count = 10 ): + def sql_load_recent_notes( self, start = 0, count = 10, reverse = False ): """ Return a SQL string to load a list of the most recently created notes within this notebook. @@ -159,7 +159,15 @@ class Notebook( Persistent ): @param start: index of recent note to start with (defaults to 0, the most recent note) @type count: int or NoneType @param count: number of recent notes to return (defaults to 10 notes) + @type reverse: bool or NoneType + @param reverse: whether to reverse the chronological order of notes. so if reverse is True, + the oldest notes are returned instead of the newest (defaults to False) """ + if reverse: + ordering = u"asc" + else: + ordering = u"desc" + return \ """ select @@ -172,9 +180,9 @@ class Notebook( Persistent ): where notebook_id = %s and note_current.id = note_creation.id order by - creation desc + creation %s limit %d offset %d; - """ % ( quote( self.object_id ), quote( self.object_id ), count, start ) + """ % ( quote( self.object_id ), quote( self.object_id ), ordering, count, start ) def sql_load_note_by_id( self, note_id ): """ diff --git a/static/js/Wiki.js b/static/js/Wiki.js index 2675f39..dd281d2 100644 --- a/static/js/Wiki.js +++ b/static/js/Wiki.js @@ -27,7 +27,7 @@ function Wiki( invoker ) { this.font_size = null; this.small_toolbar = false; this.large_toolbar_bottom = 0; - this.autosaver = Autosaver( this ); + this.autosaver = null; var total_notes_count_node = getElement( "total_notes_count" ); if ( total_notes_count_node ) @@ -70,6 +70,10 @@ function Wiki( invoker ) { this.display_message( "Luminotes support for your web browser (" + beta_agent + ") is currently in beta. If you encounter any problems, please contact support so that they can be fixed!" ); } + if ( this.notebook.read_write != NOTEBOOK_READ_WRITE_FOR_OWN_NOTES ) { + this.autosaver = Autosaver( this ); + } + var deleted_id = getElement( "deleted_id" ).value; var skip_empty_message = deleted_id ? true : false; @@ -296,13 +300,18 @@ Wiki.prototype.populate = function ( startup_notes, current_notes, note_read_wri for ( var i in current_notes ) { var note = current_notes[ i ]; + if ( !note_read_write ) + var read_write = NOTEBOOK_READ_ONLY; + else + var read_write = this.notebook.read_write; + this.create_editor( note.object_id, getElement( "static_note_" + note.object_id ).innerHTML, note.deleted_from_id, note.revision, note.creation, - this.notebook.read_write != NOTEBOOK_READ_ONLY && note_read_write, false, focus, null, + read_write, false, focus, null, note.user_id ); focus = false; @@ -317,15 +326,7 @@ Wiki.prototype.populate = function ( startup_notes, current_notes, note_read_wri if ( this.notebook.read_write != NOTEBOOK_READ_ONLY ) { connect( window, "onunload", function ( event ) { self.editor_focused( null, true ); } ); - - if ( this.notebook.read_write == NOTEBOOK_READ_WRITE || - ( this.notebook.read_write == NOTEBOOK_READ_WRITE_FOR_OWN_NOTES && - this.user.username && this.user.username != "anonymous" ) - ) - connect( "newNote", "onclick", this, "create_blank_editor" ); - else - connect( "newNote", "onclick", function( event ) { self.display_message( 'Please login first. No account? Click "sign up".' ) } ); - + connect( "newNote", "onclick", this, "create_blank_editor" ); connect( "createLink", "onclick", this, "toggle_link_button" ); if ( this.notebook.read_write == NOTEBOOK_READ_WRITE ) connect( "attachFile", "onclick", this, "toggle_attach_button" ); @@ -451,6 +452,12 @@ Wiki.prototype.background_clicked = function ( event ) { Wiki.prototype.create_blank_editor = function ( event ) { if ( event ) event.stop(); + if ( this.notebook.read_write == NOTEBOOK_READ_WRITE_FOR_OWN_NOTES && + ( !this.user.username || this.user.username == "anonymous" ) ) { + this.display_message( 'Please login first. No account? Click "sign up".' ); + return; + } + this.clear_messages(); this.clear_pulldowns(); @@ -565,6 +572,13 @@ Wiki.prototype.load_editor = function ( note_title, note_id, revision, previous_ } } + // if the notebook's read_write is NOTEBOOK_READ_WRITE_FOR_OWN_NOTES, then instead of opening + // a new post, display an error message + if ( this.notebook.read_write == NOTEBOOK_READ_WRITE_FOR_OWN_NOTES ) { + this.display_message( "No such forum post! (A forum link must point to another post in this discussion or an external web page.)" ); + return; + } + this.invoker.invoke( "/notebooks/load_note_by_title", "GET", { "notebook_id": this.notebook_id, @@ -576,6 +590,13 @@ Wiki.prototype.load_editor = function ( note_title, note_id, revision, previous_ return; } + // if the notebook's read_write is NOTEBOOK_READ_WRITE_FOR_OWN_NOTES, maintain displayed note + // order by opening an existing note on its own page + if ( this.notebook.read_write == NOTEBOOK_READ_WRITE_FOR_OWN_NOTES ) { + window.location = window.location.protocol + '//' + window.location.host + window.location.pathname + '?note_id=' + note_id; + return; + } + this.invoker.invoke( "/notebooks/load_note", "GET", { "notebook_id": this.notebook_id, @@ -789,7 +810,14 @@ Wiki.prototype.create_editor = function ( id, note_text, deleted_from_id, revisi if ( !read_write && creation ) { var short_creation = this.brief_revision( creation ); - note_text = '

' + short_creation + ' | permalink

' + note_text; + if ( user_id ) + var by = ' by ' + user_id; + else + var by = ''; + + note_text = '

Posted' + by + ' on ' + short_creation + + ' | permalink

' + note_text; } var startup = this.startup_notes[ id ]; @@ -1560,6 +1588,10 @@ Wiki.prototype.save_editor = function ( editor, fire_and_forget, callback, synch else if ( self.startup_notes[ editor.id ] ) delete self.startup_notes[ editor.id ]; + // special case to rename a NOTEBOOK_READ_WRITE_FOR_OWN_NOTES when its first note is renamed + if ( result.rank == 0 && self.notebook.read_write == NOTEBOOK_READ_WRITE_FOR_OWN_NOTES ) + self.end_notebook_rename( editor.title, true ); + if ( callback ) callback(); if ( !suppress_save_signal ) @@ -2726,8 +2758,9 @@ Wiki.prototype.start_notebook_rename = function () { notebook_name_field.select(); } -Wiki.prototype.end_notebook_rename = function () { - var new_notebook_name = getElement( "notebook_name_field" ).value; +Wiki.prototype.end_notebook_rename = function ( new_notebook_name, prevent_rename_on_click ) { + if ( !new_notebook_name ) + new_notebook_name = getElement( "notebook_name_field" ).value; // if the new name is blank or reserved, don't actually rename the notebook if ( /^\s*$/.test( new_notebook_name ) ) @@ -2739,24 +2772,32 @@ Wiki.prototype.end_notebook_rename = function () { } // rename the notebook in the header - var notebook_header_name = createDOM( - "span", - { "id": "notebook_header_name", "title": "Rename this notebook." }, - createDOM( "strong", {}, new_notebook_name ) - ); - replaceChildNodes( "notebook_header_area", notebook_header_name ); + if ( prevent_rename_on_click ) { + var notebook_header_name = createDOM( + "span", {}, + createDOM( "strong", {}, new_notebook_name ) + ); + replaceChildNodes( "notebook_header_area", notebook_header_name ); + } else { + var notebook_header_name = createDOM( + "span", + { "id": "notebook_header_name", "title": "Rename this notebook." }, + createDOM( "strong", {}, new_notebook_name ) + ); + replaceChildNodes( "notebook_header_area", notebook_header_name ); - var self = this; - connect( notebook_header_name, "onclick", function ( event ) { - self.start_notebook_rename(); - event.stop(); - } ); + var self = this; + connect( notebook_header_name, "onclick", function ( event ) { + self.start_notebook_rename(); + event.stop(); + } ); + } // rename the notebook link on the right side of the page - replaceChildNodes( - "notebook_" + this.notebook.object_id, - document.createTextNode( new_notebook_name ) - ); + var notebook_link = getElement( "notebook_" + this.notebook.object_id ); + if ( notebook_link ) { + replaceChildNodes( notebook_link, document.createTextNode( new_notebook_name ) ); + } // rename the notebook within the rss link (if any) var notebook_rss_link = getElement( "notebook_rss_link" ); diff --git a/view/Main_page.py b/view/Main_page.py index 4e5afb0..3574362 100644 --- a/view/Main_page.py +++ b/view/Main_page.py @@ -68,6 +68,7 @@ class Main_page( Page ): u"object_id" : note.object_id, u"revision" : note.revision, u"deleted_from_id" : note.deleted_from_id, + u"user_id": note.user_id, u"creation" : note.creation, } for note in notes ] @@ -89,6 +90,8 @@ class Main_page( Page ): else: updates_path = None + forum_tags = [ tag for tag in notebook.tags if tag.name == u"forum" ] + if notebook.name == u"Luminotes": notebook_path = u"/" updates_path = None # no RSS feed for the main notebook @@ -96,6 +99,9 @@ class Main_page( Page ): notebook_path = u"/guide" elif notebook.name == u"Luminotes blog": notebook_path = u"/blog" + elif forum_tags: + forum_tag = forum_tags[ 0 ] + notebook_path = u"/forums/%s/%s" % ( forum_tag.value, notebook.object_id ) else: notebook_path = u"/notebooks/%s" % notebook.object_id @@ -203,11 +209,14 @@ class Main_page( Page ): Div( id = u"deleted_notebooks", ), + self.page_navigation( notebook_path, len( notes ), total_notes_count, start, count ), + ( notebook.read_write == Notebook.READ_WRITE_FOR_OWN_NOTES ) and \ + Div( u"When you're done with your comment, click the save button to publish it.", class_ = u"small_text" ) or None, Div( Span( id = u"notes_top" ), id = u"notes", ), - ( notebook.read_write != Notebook.READ_ONLY ) and Div( + ( notebook.read_write == Notebook.READ_WRITE ) and Div( id = u"blank_note_stub", class_ = u"blank_note_stub_hidden_border", ) or None, @@ -220,17 +229,7 @@ class Main_page( Page ): u"document.getElementById( 'static_notes' ).style.display = 'none';", type = u"text/javascript", ), - # make page navigation for those notebooks that require it (such as the blog) - ( start is not None and count is not None and len( notes ) > 1 ) and Div( - ( start > 0 ) and Div( A( - u"previous page", - href = "%s?start=%d&count=%d" % ( notebook_path, max( start - count, 0 ), count ), - ) ) or None, - ( start + count < total_notes_count ) and Div( A( - u"next page", - href = "%s?start=%d&count=%d" % ( notebook_path, min( start + count, total_notes_count - 1 ), count ), - ) ) or None, - ) or None, + self.page_navigation( notebook_path, len( notes ), total_notes_count, start, count, top = False ), id = u"notebook_background", corners = ( u"tl", ), ), @@ -246,3 +245,45 @@ class Main_page( Page ): id = u"everything_area", ), ) + + def page_navigation( self, notebook_path, displayed_notes_count, total_notes_count, start, notes_per_page, top = True ): + if start is None or notes_per_page is None: + return None + + if displayed_notes_count == 1 and displayed_notes_count < total_notes_count: + if top is True: + return None + return Div( + Span( + A( + u"return to the discussion", + href = "%s" % notebook_path, + ), + ), + ) + + if start == 0 and notes_per_page >= total_notes_count: + return None + + return Div( + ( start > 0 ) and Span( + A( + u"previous", + href = "%s?start=%d&count=%d" % ( notebook_path, max( start - notes_per_page, 0 ), notes_per_page ), + ), + u" | ", + ) or None, + [ Span( + ( start == page_start ) and Strong( unicode( page_number + 1 ) ) or A( + Strong( unicode( page_number + 1 ) ), + href = "%s?start=%d&count=%d" % ( notebook_path, page_start, notes_per_page ), + ), + ) for ( page_number, page_start ) in enumerate( range( 0, total_notes_count, notes_per_page ) ) ], + ( start + notes_per_page < total_notes_count ) and Span( + u" | ", + A( + u"next", + href = "%s?start=%d&count=%d" % ( notebook_path, min( start + notes_per_page, total_notes_count - 1 ), notes_per_page ), + ), + ) or None, + )