Merge pull request #10 from jacquelineflemming/master

Issue 665 fix, to use form is_valid()

mysite/missions/svn/controllers.py

@@ -81,58 +81,6 @@ def get_password(self):
     def cat(self, path):
         return subproc_check_output(['svn', 'cat', self.file_url + path])
 
-
-class SvnCheckoutMission(object):
-    SECRET_WORD_FILE = 'word.txt'
-
-    @classmethod
-    def get_secret_word(cls, username):
-        return SvnRepository(username).cat('/trunk/' + cls.SECRET_WORD_FILE).strip()
-
-
-class SvnDiffMission(object):
-    FILE_TO_BE_PATCHED = 'README'
-    NEW_CONTENT = os.path.join(get_mission_data_path('svn'), 'README-new-for-svn-diff')
-
-    @classmethod
-    def validate_diff_and_commit_if_ok(cls, username, diff):
-        the_patch = patch.fromstring(diff)
-
-        # Check that it only patches one file.
-        if len(the_patch.hunks) != 1:
-            raise IncorrectPatch, 'The patch affects more than one file.'
-
-        # Check that the filename it patches is correct.
-        if the_patch.source[0] != cls.FILE_TO_BE_PATCHED or the_patch.target[0] != cls.FILE_TO_BE_PATCHED:
-            raise IncorrectPatch, 'The patch affects the wrong file.'
-
-        # Now we need to generate a working copy to apply the patch to.
-        # We can also use this working copy to commit the patch if it's OK.
-        wcdir = tempfile.mkdtemp()
-        repo = SvnRepository(username)
-        try:
-            subproc_check_output(['svn', 'co', repo.file_trunk_url(), wcdir])
-            file_to_patch = os.path.join(wcdir, cls.FILE_TO_BE_PATCHED)
-
-            # Check that it will apply correctly to the working copy.
-            if not the_patch._match_file_hunks(file_to_patch, the_patch.hunks[0]):
-                raise IncorrectPatch, 'The patch will not apply correctly to the latest revision.'
-
-            # Check that the resulting file matches what is expected.
-            new_content = ''.join(the_patch.patch_stream(open(file_to_patch), the_patch.hunks[0]))
-            if new_content != open(cls.NEW_CONTENT).read():
-                raise IncorrectPatch, 'The file resulting from patching does not have the correct contents.'
-
-            # Commit the patch.
-            open(file_to_patch, 'w').write(new_content)
-            commit_message = '''Fix a typo in %s.
-
-Thanks for reporting this, %s!''' % (cls.FILE_TO_BE_PATCHED, username)
-            subproc_check_output(['svn', 'commit', '-m', commit_message, '--username', 'mr_bad', wcdir])
-
-        finally:
-            shutil.rmtree(wcdir)
-
 # Helpers for the hook scripts for the svn commit mission.
 # We can mock these in the tests.
 def get_username_for_svn_txn(repo_path, txn_id):
@@ -153,7 +101,6 @@ def get_file_for_svn_txn(repo_path, txn_id, filename):
 def get_log_for_svn_txn(repo_path, txn_id):
     return subproc_check_output(['svnlook', 'log', repo_path, '-t', txn_id])
 
-
 class SvnCommitMission(object):
     SECRET_WORD_FILE = 'word.txt'
     NEW_SECRET_WORD = 'plenipotentiary'

mysite/missions/svn/forms.py

@@ -17,9 +17,73 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import django.forms
+from django.forms import ValidationError
+import os
+from mysite.missions.base.controllers import get_mission_data_path
+import patch
+from mysite.missions.svn import controllers
 
 class CheckoutForm(django.forms.Form):
     secret_word = django.forms.CharField(error_messages={'required': 'No secret word was given.'})
+    SECRET_WORD_FILE = 'word.txt'
+
+    def __init__(self, username=None, *args, **kwargs):
+        super(CheckoutForm, self).__init__(*args, **kwargs)
+        self.username = username 
+
+    def clean_secret_word(self):
+        cat_trunk = controllers.SvnRepository(self.username).cat('/trunk/' +
+                                                     self.SECRET_WORD_FILE).strip()
+        if self.cleaned_data['secret_word'] != cat_trunk:
+            raise ValidationError, 'The secret word is incorrect.'
+
 
 class DiffForm(django.forms.Form):
     diff = django.forms.CharField(error_messages={'required': 'No svn diff output was given.'}, widget=django.forms.Textarea())
+    FILE_TO_BE_PATCHED = 'README'
+    NEW_CONTENT = os.path.join(get_mission_data_path('svn'), 'README-new-for-svn-diff')
+
+    def __init__(self, username=None, wcdir=None, request=None, *args, **kwargs):
+        super(DiffForm, self).__init__(request, *args, **kwargs)
+        self.username = username 
+        self.wcdir = wcdir
+        if not wcdir == None:
+            self.file_to_patch = os.path.join(wcdir, self.FILE_TO_BE_PATCHED)
+
+    def clean_diff(self):
+        """Validate the diff form. 
+        This function will be invoked by django.form.Forms.is_valid(), and will raise the exception
+        ValidationError
+
+        """
+        self.the_patch = patch.fromstring(self.cleaned_data['diff'])
+        # Check that it only patches one file.
+        if len(self.the_patch.hunks) != 1:
+            raise ValidationError, 'The patch affects more than one file.'
+
+        # Check that the filename it patches is correct.
+        if self.the_patch.source[0] != self.FILE_TO_BE_PATCHED or self.the_patch.target[0] != self.FILE_TO_BE_PATCHED:
+            raise ValidationError, 'The patch affects the wrong file.'
+
+        # Now we need to generate a working copy to apply the patch to.
+        # We can also use this working copy to commit the patch if it's OK.
+        repo = controllers.SvnRepository(self.username)
+        controllers.subproc_check_output(['svn', 'co', repo.file_trunk_url(), self.wcdir])
+
+        # Check that it will apply correctly to the working copy.
+        if not self.the_patch._match_file_hunks(self.file_to_patch, self.the_patch.hunks[0]):
+            raise ValidationError, 'The patch will not apply correctly to the lastest revision.' 
+
+        # Check that the resulting file matches what is expected.
+        self.new_content = ''.join(self.the_patch.patch_stream(open(self.file_to_patch), self.the_patch.hunks[0]))
+        if self.new_content != open(self.NEW_CONTENT).read():
+            raise ValidationError, 'The file resulting from patching does not have the correct contents.'
+
+    def commit_diff(self):
+        '''Commit the diff form.'''
+        # Commit the patch.
+        open(self.file_to_patch, 'w').write(self.new_content)
+        commit_message = '''Fix a typo in %s.
+
+Thanks for reporting this, %s!''' % (self.FILE_TO_BE_PATCHED, self.username)
+        controllers.subproc_check_output(['svn', 'commit', '-m', commit_message, '--username', 'mr_bad', self.wcdir])

mysite/missions/svn/tests.py

@@ -33,6 +33,7 @@
 from mysite.missions.svn import views, controllers
 from django.utils.unittest import skipIf
 import mysite.base.depends
+from mysite.missions.svn.forms import DiffForm
 
 @skipIf(not mysite.base.depends.svnadmin_available(),
         "Skipping tests for Subversion training mission for now. To run these tests, install the 'subversion' package in your package manager.")
@@ -111,8 +112,9 @@ def test_do_diff_mission_correctly(self):
         try:
             # Check the repository out and make the required change.
             subprocess.check_call(['svn', 'checkout', response.context['checkout_url'], checkoutdir])
-            new_contents = open(os.path.join(controllers.get_mission_data_path('svn'), controllers.SvnDiffMission.NEW_CONTENT)).read()
-            open(os.path.join(checkoutdir, controllers.SvnDiffMission.FILE_TO_BE_PATCHED), 'w').write(new_contents)
+            new_contents = open(os.path.join(controllers.get_mission_data_path('svn'),
+                              DiffForm.NEW_CONTENT)).read()
+            open(os.path.join(checkoutdir, DiffForm.FILE_TO_BE_PATCHED), 'w').write(new_contents)
 
             # Make the diff.
             diff = subproc_check_output(['svn', 'diff'], cwd=checkoutdir)
@@ -135,8 +137,9 @@ def test_do_diff_mission_correctly_except_omit_the_final_whitespace(self):
         try:
             # Check the repository out and make the required change.
             subprocess.check_call(['svn', 'checkout', response.context['checkout_url'], checkoutdir])
-            new_contents = open(os.path.join(controllers.get_mission_data_path('svn'), controllers.SvnDiffMission.NEW_CONTENT)).read()
-            open(os.path.join(checkoutdir, controllers.SvnDiffMission.FILE_TO_BE_PATCHED), 'w').write(new_contents)
+            new_contents = open(os.path.join(controllers.get_mission_data_path('svn'),
+                              DiffForm.NEW_CONTENT)).read()
+            open(os.path.join(checkoutdir, DiffForm.FILE_TO_BE_PATCHED), 'w').write(new_contents)
 
             # Make the diff.
             diff = subproc_check_output(['svn', 'diff'], cwd=checkoutdir)

mysite/missions/svn/views.py

@@ -20,6 +20,9 @@
 from mysite.missions.base.views import *
 from mysite.missions.svn import forms, controllers
 import mysite.missions.base.views
+import shutil
+import tempfile
+from django.shortcuts import render
 
 ### POST handlers
 ###
@@ -43,17 +46,18 @@ def resetrepo(request):
 def diff_submit(request):
     # Initialize data array and some default values.
     data = {}
-    data['svn_diff_form'] = forms.DiffForm()
+    data['svn_diff_form'] = forms.DiffForm(request.user.username)
     data['svn_diff_error_message'] = ''
     if request.method == 'POST':
-        form = forms.DiffForm(request.POST)
-        if form.is_valid():
-            try:
-                controllers.SvnDiffMission.validate_diff_and_commit_if_ok(request.user.username, form.cleaned_data['diff'])
+        wcdir = tempfile.mkdtemp()
+        try:
+            form = forms.DiffForm(request.user.username, wcdir, request.POST)
+            if form.is_valid():
+                form.commit_diff()
                 controllers.set_mission_completed(request.user.get_profile(), 'svn_diff')
                 return HttpResponseRedirect(reverse('svn_diff'))
-            except controllers.IncorrectPatch, e:
-                data['svn_diff_error_message'] = str(e)
+        finally:
+            shutil.rmtree(wcdir)
         data['svn_diff_form'] = form
     request.method = 'GET'
     return Diff.as_view()(request, extra_context_data=data)
@@ -62,20 +66,19 @@ def diff_submit(request):
 def checkout_submit(request):
     # Initialize data array and some default values.
     data = {}
-    data['svn_checkout_form'] = forms.CheckoutForm()
+    data['svn_checkout_form'] = forms.CheckoutForm(request.user.username)
     data['svn_checkout_error_message'] = ''
     if request.method == 'POST':
-        form = forms.CheckoutForm(request.POST)
+        form = forms.CheckoutForm(request.user.username, request.POST)
         if form.is_valid():
-            if form.cleaned_data['secret_word'] == controllers.SvnCheckoutMission.get_secret_word(request.user.username):
-                controllers.set_mission_completed(request.user.get_profile(), 'svn_checkout')
-                return HttpResponseRedirect(reverse('svn_checkout'))
-            else:
-                data['svn_checkout_error_message'] = 'The secret word is incorrect.'
+            controllers.set_mission_completed(request.user.get_profile(), 'svn_checkout')
+            return HttpResponseRedirect(reverse('svn_checkout'))
         data['svn_checkout_form'] = form
+
     # If we get here, just hack up the request object to pretend it is a GET
     # so the dispatch system in the class-based view can use the GET handler.
     request.method = 'GET'
+
     return Checkout.as_view()(request, extra_context_data=data)
 
 ### All GET handlers are subclasses of this so that tedious template
@@ -100,8 +103,8 @@ def get_context_data(self, *args, **kwargs):
             if new_data['repository_exists']:
                 new_data.update({
                         'checkout_url': repo.public_trunk_url(),
-                        'secret_word_file': controllers.SvnCheckoutMission.SECRET_WORD_FILE,
-                        'file_for_svn_diff': controllers.SvnDiffMission.FILE_TO_BE_PATCHED,
+                        'secret_word_file': forms.CheckoutForm.SECRET_WORD_FILE,
+                        'file_for_svn_diff': forms.DiffForm.FILE_TO_BE_PATCHED,
                         'new_secret_word': controllers.SvnCommitMission.NEW_SECRET_WORD,
                         'commit_username': self.request.user.username,
                         'commit_password': repo.get_password()})
@@ -125,9 +128,13 @@ class Checkout(SvnBaseView):
 
     def get_context_data(self, *args, **kwargs):
         data = super(Checkout, self).get_context_data(*args, **kwargs)
-        data['svn_checkout_form'] = forms.CheckoutForm()
+        if kwargs.has_key('extra_context_data'):
+            data.update(kwargs['extra_context_data'])
+        else:
+            data['svn_checkout_form'] = forms.CheckoutForm()
         return data
 
+
 class Diff(SvnBaseView):
     login_required = True
     this_mission_page_short_name = 'Diffing your changes'
@@ -136,7 +143,10 @@ class Diff(SvnBaseView):
 
     def get_context_data(self, *args, **kwargs):
         data = super(Diff, self).get_context_data(*args, **kwargs)
-        data['svn_diff_form'] = forms.DiffForm()
+        if kwargs.has_key('extra_context_data'):
+            data.update(kwargs['extra_context_data']) 
+        else:
+            data['svn_diff_form'] = forms.DiffForm()
         return data
 
 class Commit(SvnBaseView):

mysite/missions/templates/missions/svn/checkout.html

@@ -41,9 +41,6 @@
 <form method="post" action="{% url mysite.missions.svn.views.checkout_submit %}#svncheckout-form">{% csrf_token %}
   <div class="form-row">
     <a name="svncheckout-form"></a>
-    {% if svn_checkout_error_message %}
-    <ul class="errorlist"><li>{{ svn_checkout_error_message }}</li></ul>
-    {% endif %}
     {{ svn_checkout_form.secret_word.errors }}
     <br style="clear: left;" />
     Secret word: <br style="clear: left;" />

mysite/missions/templates/missions/svn/diff.html

@@ -48,9 +48,6 @@
 <form method="post" action="{% url mysite.missions.svn.views.diff_submit %}#svndiff-form">{% csrf_token %}
   <div class="form-row">
     <a name="svndiff-form"></a>
-    {% if svn_diff_error_message %}
-    <ul class="errorlist"><li>{{ svn_diff_error_message }}</li></ul>
-    {% endif %}
     {{ svn_diff_form.diff.errors }}
     <br style="clear: left;" />
     <code>svn diff</code> output: <br style="clear: left;" />