From 44e2da2fe60a3b8765d0fef5a9ce0c3e5997dd01 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Sun, 12 Jun 2011 03:24:31 +0200 Subject: [PATCH 01/10] Added Markdown rendering for `media_entry` --- mediagoblin/db/models.py | 3 ++- mediagoblin/edit/views.py | 9 ++++++++- mediagoblin/submit/views.py | 7 +++++++ mediagoblin/templates/mediagoblin/user_pages/media.html | 4 +++- mediagoblin/user_pages/views.py | 4 ++-- mediagoblin/util.py | 3 +-- setup.py | 1 + 7 files changed, 24 insertions(+), 7 deletions(-) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 3da97a49..0b092d60 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -73,7 +73,8 @@ class MediaEntry(Document): 'title': unicode, 'slug': unicode, 'created': datetime.datetime, - 'description': unicode, + 'description': unicode, # May contain markdown/up + 'description_html': unicode, # May contain plaintext, or HTML 'media_type': unicode, 'media_data': dict, # extra data relevant to this media_type 'plugin_data': dict, # plugins can dump stuff here. diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index c5f0f435..2bc53a54 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -47,7 +47,14 @@ def edit_media(request, media): u'An entry with that slug already exists for this user.') else: media['title'] = request.POST['title'] - media['description'] = request.POST['description'] + media['description'] = request.POST.get('description') + + import markdown + md = markdown.Markdown( + safe_mode = 'escape') + media['description_html'] = md.convert( + media['description']) + media['slug'] = request.POST['slug'] media.save() diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index e9b5c37e..21562e6f 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -48,6 +48,13 @@ def submit_start(request): entry = request.db.MediaEntry() entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) entry['description'] = request.POST.get('description') + + import markdown + md = markdown.Markdown( + safe_mode = 'escape') + entry['description_html'] = md.convert( + entry['description']) + entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] diff --git a/mediagoblin/templates/mediagoblin/user_pages/media.html b/mediagoblin/templates/mediagoblin/user_pages/media.html index 200f13cd..44bc38b8 100644 --- a/mediagoblin/templates/mediagoblin/user_pages/media.html +++ b/mediagoblin/templates/mediagoblin/user_pages/media.html @@ -25,7 +25,9 @@ -

{{ media.description }}

+ {% autoescape False %} +

{{ media.description_html }}

+ {% endautoescape %}

Uploaded on {{ "%4d-%02d-%02d"|format(media.created.year, media.created.month, media.created.day) }} diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py index 323c3e54..4ec0f0aa 100644 --- a/mediagoblin/user_pages/views.py +++ b/mediagoblin/user_pages/views.py @@ -81,10 +81,10 @@ def atom_feed(request): feed = AtomFeed(request.matchdict['user'], feed_url=request.url, url=request.host_url) - + for entry in cursor: feed.add(entry.get('title'), - entry.get('description'), + entry.get('description_html'), content_type='html', author=request.matchdict['user'], updated=entry.get('created'), diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 64e21ca9..1e8fa095 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -34,7 +34,6 @@ from webob import Response, exc from mediagoblin import globals as mgoblin_globals from mediagoblin.db.util import ObjectId - TESTS_ENABLED = False def _activate_testing(): """ @@ -99,7 +98,7 @@ def get_jinja_env(template_loader, locale): template_env = jinja2.Environment( loader=template_loader, autoescape=True, - extensions=['jinja2.ext.i18n']) + extensions=['jinja2.ext.i18n', 'jinja2.ext.autoescape']) template_env.install_gettext_callables( mgoblin_globals.translations.gettext, diff --git a/setup.py b/setup.py index 46da7276..fb86d600 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ setup( 'translitcodec', 'argparse', 'webtest', + 'Markdown', ], test_suite='nose.collector', From 44e51d3464e719e596e1480b7af2957742a9085b Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Wed, 15 Jun 2011 23:07:54 +0200 Subject: [PATCH 02/10] Made changes according to http://bugs.foocorp.net/issues/363#note-5 --- mediagoblin/edit/views.py | 10 ++++++---- mediagoblin/submit/views.py | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index 2bc53a54..6c16a61e 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -17,11 +17,13 @@ from webob import exc -from mediagoblin.util import render_to_response, redirect +from mediagoblin.util import render_to_response, redirect, clean_html from mediagoblin.edit import forms from mediagoblin.edit.lib import may_edit_media from mediagoblin.decorators import require_active_login, get_user_media_entry +import markdown + @get_user_media_entry @require_active_login @@ -49,11 +51,11 @@ def edit_media(request, media): media['title'] = request.POST['title'] media['description'] = request.POST.get('description') - import markdown md = markdown.Markdown( safe_mode = 'escape') - media['description_html'] = md.convert( - media['description']) + media['description_html'] = clean_html( + md.convert( + media['description'])) media['slug'] = request.POST['slug'] media.save() diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 21562e6f..437a5a51 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -19,11 +19,13 @@ from cgi import FieldStorage from werkzeug.utils import secure_filename -from mediagoblin.util import render_to_response, redirect +from mediagoblin.util import render_to_response, redirect, clean_html from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security from mediagoblin.process_media import process_media_initial +import markdown + @require_active_login def submit_start(request): @@ -49,11 +51,11 @@ def submit_start(request): entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) entry['description'] = request.POST.get('description') - import markdown md = markdown.Markdown( safe_mode = 'escape') - entry['description_html'] = md.convert( - entry['description']) + entry['description_html'] = clean_html( + md.convert( + entry['description'])) entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] From 0a4cecdc6603b02534c591d065ec772a0f723c8b Mon Sep 17 00:00:00 2001 From: Chris Moylan Date: Sun, 19 Jun 2011 00:22:47 -0500 Subject: [PATCH 03/10] Added tests for all sorts of login form abuse. Added tests for log out --- mediagoblin/tests/test_auth.py | 82 +++++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index b8389f8d..1b3b5082 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -242,17 +242,69 @@ def test_authentication_views(test_app): test_user.save() # Get login + # --------- test_app.get('/auth/login/') - # Make sure it rendered with the appropriate template assert util.TEMPLATE_TEST_CONTEXT.has_key( 'mediagoblin/auth/login.html') - # Log in as that user + # Failed login - blank form + # ------------------------- + util.clear_test_template_context() + response = test_app.post('/auth/login/') + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.username.errors == [u'This field is required.'] + assert form.password.errors == [u'This field is required.'] + + # Failed login - blank user + # ------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'password': u'toast'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.username.errors == [u'This field is required.'] + + # Failed login - blank password + # ----------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.password.errors == [u'This field is required.'] + + # Failed login - bad user + # ----------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'steve', + 'password': 'toast'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + assert context['login_failed'] + + # Failed login - bad password + # --------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris', + 'password': 'jam'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + assert context['login_failed'] + + # Successful login + # ---------------- util.clear_test_template_context() response = test_app.post( '/auth/login/', { 'username': u'chris', 'password': 'toast'}) + + # User should be redirected response.follow() assert_equal( urlparse.urlsplit(response.location)[2], @@ -260,10 +312,28 @@ def test_authentication_views(test_app): assert util.TEMPLATE_TEST_CONTEXT.has_key( 'mediagoblin/root.html') - # Make sure we're in the session or something - session = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html']['request'].session + # Make sure user is in the session + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html'] + session = context['request'].session assert session['user_id'] == unicode(test_user['_id']) - # Log out as that user - # Make sure we're not in the session + # TODO: test custom redirect when next=True + + # Successful logout + # ----------------- + util.clear_test_template_context() + response = test_app.get('/auth/logout/') + + # Should be redirected to index page + response.follow() + assert_equal( + urlparse.urlsplit(response.location)[2], + '/') + assert util.TEMPLATE_TEST_CONTEXT.has_key( + 'mediagoblin/root.html') + + # Make sure the user is not in the session + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html'] + session = context['request'].session + assert session.has_key('user_id') == False From 12c231c8acc2036330143baa17610b6faa901ad6 Mon Sep 17 00:00:00 2001 From: Chris Moylan Date: Sun, 19 Jun 2011 12:28:53 -0500 Subject: [PATCH 04/10] added test coverage for redirecting after login with the next param --- mediagoblin/tests/test_auth.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index 1b3b5082..3a13cbb1 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -317,8 +317,6 @@ def test_authentication_views(test_app): session = context['request'].session assert session['user_id'] == unicode(test_user['_id']) - # TODO: test custom redirect when next=True - # Successful logout # ----------------- util.clear_test_template_context() @@ -337,3 +335,15 @@ def test_authentication_views(test_app): session = context['request'].session assert session.has_key('user_id') == False + # User is redirected to custom URL if POST['next'] is set + # ------------------------------------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris', + 'password': 'toast', + 'next' : '/u/chris/'}) + assert_equal( + urlparse.urlsplit(response.location)[2], + '/u/chris/') + From 8b7663a0bd04da856d36b21d219c396c4111a06b Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:52:27 -0500 Subject: [PATCH 05/10] Making ./runtests.sh the official way to run the tests. --- docs/hackinghowto.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index fcab5844..b03efa91 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -158,7 +158,7 @@ Running the test suite Run:: - ./bin/nosetests + ./runtests.sh Running a shell From 4bf8e8888c7df6717eb43487136cc9d5c155bc6c Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:41:40 -0500 Subject: [PATCH 06/10] Adds util.cleaned_markdown_conversion() and uses it in the submission process This simplifies the markdown processing & html cleaning of descritions and etc by providing a wrapper function that we can use in multiple locations. --- mediagoblin/submit/views.py | 18 ++++++++---------- mediagoblin/util.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 437a5a51..6139614e 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -19,13 +19,12 @@ from cgi import FieldStorage from werkzeug.utils import secure_filename -from mediagoblin.util import render_to_response, redirect, clean_html +from mediagoblin.util import ( + render_to_response, redirect, cleaned_markdown_conversion) from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security from mediagoblin.process_media import process_media_initial -import markdown - @require_active_login def submit_start(request): @@ -48,14 +47,13 @@ def submit_start(request): # create entry and save in database entry = request.db.MediaEntry() - entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) + entry['title'] = ( + request.POST['title'] + or unicode(splitext(filename)[0])) + entry['description'] = request.POST.get('description') - - md = markdown.Markdown( - safe_mode = 'escape') - entry['description_html'] = clean_html( - md.convert( - entry['description'])) + entry['description_html'] = cleaned_markdown_conversion( + entry['description']) entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 4d625728..0e43a1f5 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -29,6 +29,7 @@ import jinja2 import translitcodec from webob import Response, exc from lxml.html.clean import Cleaner +import markdown from mediagoblin import mg_globals from mediagoblin.db.util import ObjectId @@ -375,6 +376,16 @@ def clean_html(html): return HTML_CLEANER.clean_html(html) +MARKDOWN_INSTANCE = markdown.Markdown(safe_mode='escape') + + +def cleaned_markdown_conversion(text): + """ + Take a block of text, run it through MarkDown, and clean its HTML. + """ + return clean_html(MARKDOWN_INSTANCE.convert(text)) + + SETUP_GETTEXTS = {} def setup_gettext(locale): From a01d04a017cbf6009e7122b123aaef7697e299c4 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:42:48 -0500 Subject: [PATCH 07/10] Provide a migration to add description_html to MediaEntries that don't have it --- mediagoblin/db/migrations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index f1f625b7..b87988fe 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from mediagoblin.util import cleaned_markdown_conversion + from mongokit import DocumentMigration @@ -33,5 +35,19 @@ class MediaEntryMigration(DocumentMigration): self.collection.update( self.target, self.update, multi=True, safe=True) + def allmigration02_add_description_html(self): + """ + Now that we can have rich descriptions via Markdown, we should + update all existing entries to record the rich description versions. + """ + self.target = {'description_html': {'$exists': False}} + + if not self.status: + for doc in self.collection.find(self.target): + self.update = { + '$set': { + 'description_html': cleaned_markdown_conversion( + doc['description'])}} + MIGRATE_CLASSES = ['MediaEntry'] From fcdd172264fcc8b312fe449301471fe1761e2ed3 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:53:38 -0500 Subject: [PATCH 08/10] Updated git documentation to have more useful branch names. I haven't discussed this with Will yet... if he gets unhappy we can roll back this documentation change :) --- docs/git.rst | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/git.rst b/docs/git.rst index c3f7ccce..2836ecd8 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -108,8 +108,8 @@ Contributing changes -------------------- Slartibartfast from the planet Magrathea far off in the universe has -decided that he is bored with fjords and wants to fix issue 42 and -send us the changes. +decided that he is bored with fjords and wants to fix issue 42 (the +meaning of life bug) and send us the changes. Slartibartfast has cloned the MediaGoblin repository and his clone lives on gitorious. @@ -125,18 +125,18 @@ Slartibartfast does the following: git fetch --all -p 2. Creates a branch from the tip of the MediaGoblin repository (the - remote is named ``gmg``) master branch called ``issue_42``:: + remote is named ``gmg``) master branch called ``bug42_meaning_of_life``:: - git checkout -b issue_42 gmg/master + git checkout -b bug42_meaning_of_life gmg/master -3. Slartibartfast works hard on his changes in the ``issue_42`` +3. Slartibartfast works hard on his changes in the ``bug42_meaning_of_life`` branch. When done, he wants to notify us that he has made changes he wants us to see. 4. Slartibartfast pushes his changes to his clone (the remote is named ``origin``):: - git push origin issue_42 --set-upstream + git push origin bug42_meaning_of_life --set-upstream 5. Slartibartfast adds a comment to issue 42 with the url for his repository and the name of the branch he put the code in. He also @@ -155,19 +155,19 @@ He runs the unit tests and discovers there's a bug in the code! Then he does this: -1. He checks out the ``issue_42`` branch:: +1. He checks out the ``bug42_meaning_of_life`` branch:: - git checkout issue_42 + git checkout bug42_meaning_of_life -2. He fixes the bug and checks it into the ``issue_42`` branch. +2. He fixes the bug and checks it into the ``bug42_meaning_of_life`` branch. 3. He pushes his changes to his clone (the remote is named ``origin``):: - git push origin issue_42 + git push origin bug42_meaning_of_life 4. He adds another comment to issue 42 explaining about the mistake and how he fixed it and that he's pushed the new change to the - ``issue_42`` branch of his publicly available clone. + ``bug42_meaning_of_life`` branch of his publicly available clone. What happens next @@ -180,7 +180,7 @@ request with his changes and explains what they are. Later, someone checks out his code and finds a problem with it. He adds a comment to the issue tracker specifying the problem and asks Slartibartfast to fix it. Slartibartfst goes through the above steps -again, fixes the issue, pushes it to his ``issue_42`` branch and adds +again, fixes the issue, pushes it to his ``bug42_meaning_of_life`` branch and adds another comment to the issue tracker about how he fixed it. Later, someone checks out his code and is happy with it. Someone @@ -192,8 +192,8 @@ Slartibartfast is notified of this. Slartibartfast does a:: git fetch --all The changes show up in the ``master`` branch of the ``gmg`` remote. -Slartibartfast now deletes his ``issue_42`` branch because he doesn't -need it anymore. +Slartibartfast now deletes his ``bug42_meaning_of_life`` branch +because he doesn't need it anymore. How to learn git From 41f965c59d0f0673ac6e9f373e60226b3e768559 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 20 Jun 2011 08:55:58 -0500 Subject: [PATCH 09/10] Moving server.ini->paste.ini because willkg points out server.ini is ambiguous Updating the file, updating lazyserver.sh, updating docs. --- docs/hackinghowto.rst | 2 +- lazyserver.sh | 2 +- server.ini => paste.ini | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename server.ini => paste.ini (100%) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index b03efa91..911f2340 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -136,7 +136,7 @@ This is fine in development, but if you want to actually run celery separately for testing (or deployment purposes), you'll want to run the server independently:: - ./bin/paster serve server.ini --reload + ./bin/paster serve paste.ini --reload Running celeryd diff --git a/lazyserver.sh b/lazyserver.sh index bd93b109..fdb03ba0 100755 --- a/lazyserver.sh +++ b/lazyserver.sh @@ -27,4 +27,4 @@ else exit 1 fi -CELERY_ALWAYS_EAGER=true $PASTER serve server.ini --reload +CELERY_ALWAYS_EAGER=true $PASTER serve paste.ini --reload diff --git a/server.ini b/paste.ini similarity index 100% rename from server.ini rename to paste.ini From 5c441e75ebc4ad63c3a5362d9bc451abe97984d2 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 20 Jun 2011 08:57:58 -0500 Subject: [PATCH 10/10] Also moving the test_server.ini to test_paste.ini to avoid ambiguity. --- mediagoblin/tests/{test_server.ini => test_paste.ini} | 0 mediagoblin/tests/tools.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename mediagoblin/tests/{test_server.ini => test_paste.ini} (100%) diff --git a/mediagoblin/tests/test_server.ini b/mediagoblin/tests/test_paste.ini similarity index 100% rename from mediagoblin/tests/test_server.ini rename to mediagoblin/tests/test_paste.ini diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index 515bccd4..9e36fc5c 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -30,7 +30,7 @@ from mediagoblin.db.open import setup_connection_and_db_from_config MEDIAGOBLIN_TEST_DB_NAME = '__mediagoblinunittests__' TEST_SERVER_CONFIG = pkg_resources.resource_filename( - 'mediagoblin.tests', 'test_server.ini') + 'mediagoblin.tests', 'test_paste.ini') TEST_APP_CONFIG = pkg_resources.resource_filename( 'mediagoblin.tests', 'test_mgoblin_app.ini') TEST_USER_DEV = pkg_resources.resource_filename(