From 3fd754f12df42d173b6e9a93e7c32f8f5be55e7f Mon Sep 17 00:00:00 2001 From: Markus Amalthea Magnuson Date: Fri, 5 Jun 2015 01:06:03 +0100 Subject: [PATCH] Fixed #24907 -- Updated contributing tutorial with a more recent example ticket. --- docs/intro/contributing.txt | 403 ++++++++++++++++++------------------ docs/spelling_wordlist | 2 + 2 files changed, 205 insertions(+), 200 deletions(-) diff --git a/docs/intro/contributing.txt b/docs/intro/contributing.txt index 7b77b6587e..fb7fafaebb 100644 --- a/docs/intro/contributing.txt +++ b/docs/intro/contributing.txt @@ -97,15 +97,19 @@ The first step to contributing to Django is to get a copy of the source code. From the command line, use the ``cd`` command to navigate to the directory where you'll want your local copy of Django to live. -Download the Django source code repository using the following command:: +Download the Django source code repository using the following command: - git clone https://github.com/django/django.git +.. code-block:: console + + $ git clone https://github.com/django/django.git .. note:: - For users who wish to use `virtualenv`__, you can use:: + For users who wish to use `virtualenv`__, you can use: - pip install -e /path/to/your/local/clone/django/ + .. code-block:: console + + $ pip install -e /path/to/your/local/clone/django/ (where ``django`` is the directory of your clone that contains ``setup.py``) to link your cloned checkout into a virtual environment. This @@ -117,7 +121,7 @@ __ http://www.virtualenv.org Rolling back to a previous revision of Django ============================================= -For this tutorial, we'll be using ticket :ticket:`17549` as a case study, so we'll +For this tutorial, we'll be using ticket :ticket:`24788` as a case study, so we'll rewind Django's version history in git to before that ticket's patch was applied. This will allow us to go through all of the steps involved in writing that patch from scratch, including running Django's test suite. @@ -128,36 +132,46 @@ development revision of Django when working on your own patch for a ticket!** .. note:: - The patch for this ticket was written by Ulrich Petri, and it was applied - to Django as `commit ac2052ebc84c45709ab5f0f25e685bf656ce79bc`__. + The patch for this ticket was written by Paweł Marczewski, and it was + applied to Django as `commit 4df7e8483b2679fc1cba3410f08960bac6f51115`__. Consequently, we'll be using the revision of Django just prior to that, - `commit 39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac`__. + `commit 4ccfc4439a7add24f8db4ef3960d02ef8ae09887`__. -__ https://github.com/django/django/commit/ac2052ebc84c45709ab5f0f25e685bf656ce79bc -__ https://github.com/django/django/commit/39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac +__ https://github.com/django/django/commit/4df7e8483b2679fc1cba3410f08960bac6f51115 +__ https://github.com/django/django/commit/4ccfc4439a7add24f8db4ef3960d02ef8ae09887 Navigate into Django's root directory (that's the one that contains ``django``, ``docs``, ``tests``, ``AUTHORS``, etc.). You can then check out the older -revision of Django that we'll be using in the tutorial below:: +revision of Django that we'll be using in the tutorial below: - git checkout 39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac +.. code-block:: console + + $ git checkout 4ccfc4439a7add24f8db4ef3960d02ef8ae09887 Running Django's test suite for the first time ============================================== When contributing to Django it's very important that your code changes don't -introduce bugs into other areas of Django. One way to check that Django still +introduce bugs into other areas of Django. One way to check that Django still works after you make your changes is by running Django's test suite. If all the tests still pass, then you can be reasonably sure that your changes haven't completely broken Django. If you've never run Django's test suite before, it's a good idea to run it once beforehand just to get familiar with what its output is supposed to look like. -We can run the test suite by simply ``cd``-ing into the Django ``tests/`` -directory and, if you're using GNU/Linux, Mac OS X or some other flavor of -Unix, run:: +Before running the test suite, install its dependencies by first ``cd``-ing +into the Django ``tests/`` directory and then running: - PYTHONPATH=.. python runtests.py --settings=test_sqlite +.. code-block:: console + + $ pip install -r requirements/py3.txt # or py2.txt if you are running Python 2 + +Now we are ready to run the test suite. If you're using GNU/Linux, Mac OS X or +some other flavor of Unix, run: + +.. code-block:: console + + $ PYTHONPATH=.. ./runtests.py If you're on Windows, the above should work provided that you are using "Git Bash" provided by the default Git install. GitHub has a `nice tutorial`__. @@ -171,7 +185,7 @@ __ https://help.github.com/articles/set-up-git#platform-windows of ``tests``. ``virtualenv`` puts your copy of Django on the ``PYTHONPATH`` automatically. -Now sit back and relax. Django's entire test suite has over 4800 different +Now sit back and relax. Django's entire test suite has over 9,600 different tests, so it can take anywhere from 5 to 15 minutes to run, depending on the speed of your computer. @@ -234,56 +248,42 @@ Now for our hands-on example. __ http://en.wikipedia.org/wiki/Test-driven_development -Writing some tests for ticket #17549 +Writing some tests for ticket #24788 ------------------------------------ -Ticket :ticket:`17549` describes the following, small feature addition: +Ticket :ticket:`24788` proposes a small feature addition: the ability to +specify the class level attribute ``prefix`` on Form classes, so that:: - It's useful for URLField to give you a way to open the URL; otherwise you - might as well use a CharField. + […] forms which ship with apps could effectively namespace themselves such + that N overlapping form fields could be POSTed at once and resolved to the + correct form. -In order to resolve this ticket, we'll add a ``render`` method to the -``AdminURLFieldWidget`` in order to display a clickable link above the input -widget. Before we make those changes though, we're going to write a couple -tests to verify that our modification functions correctly and continues to -function correctly in the future. +In order to resolve this ticket, we'll add a ``prefix`` attribute to the +``BaseForm`` class. When creating instances of this class, passing a prefix to +the ``__init__()`` method will still set that prefix on the created instance. +But not passing a prefix (or passing ``None``) will use the class-level prefix. +Before we make those changes though, we're going to write a couple tests to +verify that our modification functions correctly and continues to function +correctly in the future. -Navigate to Django's ``tests/regressiontests/admin_widgets/`` folder and -open the ``tests.py`` file. Add the following code on line 269 right before the -``AdminFileWidgetTest`` class:: +Navigate to Django's ``tests/forms_tests/tests/`` folder and open the +``test_forms.py`` file. Add the following code on line 1674 right before the +``test_forms_with_null_boolean`` function:: - class AdminURLWidgetTest(DjangoTestCase): - def test_render(self): - w = widgets.AdminURLFieldWidget() - self.assertHTMLEqual( - conditional_escape(w.render('test', '')), - '' - ) - self.assertHTMLEqual( - conditional_escape(w.render('test', 'http://example.com')), - '

Currently:http://example.com
Change:

' - ) + def test_class_prefix(self): + # Prefix can be also specified at the class level. + class Person(Form): + first_name = CharField() + prefix = 'foo' - def test_render_idn(self): - w = widgets.AdminURLFieldWidget() - self.assertHTMLEqual( - conditional_escape(w.render('test', 'http://example-äüö.com')), - '

Currently:http://example-äüö.com
Change:

' - ) + p = Person() + self.assertEqual(p.prefix, 'foo') - def test_render_quoting(self): - w = widgets.AdminURLFieldWidget() - self.assertHTMLEqual( - conditional_escape(w.render('test', 'http://example.com/some text')), - '

Currently:http://example.com/<sometag>some text</sometag>
Change:

' - ) - self.assertHTMLEqual( - conditional_escape(w.render('test', 'http://example-äüö.com/some text')), - '

Currently:http://example-äüö.com/<sometag>some text</sometag>
Change:

' - ) + p = Person(prefix='bar') + self.assertEqual(p.prefix, 'bar') -The new tests check to see that the ``render`` method we'll be adding works -correctly in a couple different situations. +This new test checks that setting a class level prefix works as expected, and +that passing a ``prefix`` parameter when creating an instance still works too. .. admonition:: But this testing thing looks kinda hard... @@ -304,68 +304,67 @@ __ https://docs.python.org/library/unittest.html Running your new test --------------------- -Remember that we haven't actually made any modifications to -``AdminURLFieldWidget`` yet, so our tests are going to fail. Let's run all the -tests in the ``model_forms_regress`` folder to make sure that's really what -happens. From the command line, ``cd`` into the Django ``tests/`` directory -and run:: +Remember that we haven't actually made any modifications to ``BaseForm`` yet, +so our tests are going to fail. Let's run all the tests in the ``forms_tests`` +folder to make sure that's really what happens. From the command line, ``cd`` +into the Django ``tests/`` directory and run: - PYTHONPATH=.. python runtests.py --settings=test_sqlite admin_widgets +.. code-block:: console -If the tests ran correctly, you should see three failures corresponding to each -of the test methods we added. If all of the tests passed, then you'll want to -make sure that you added the new test shown above to the appropriate folder and -class. + $ PYTHONPATH=.. ./runtests.py forms_tests + +If the tests ran correctly, you should see one failure corresponding to the test +method we added. If all of the tests passed, then you'll want to make sure that +you added the new test shown above to the appropriate folder and class. Writing the code for your ticket ================================ -Next we'll be adding the functionality described in ticket :ticket:`17549` to +Next we'll be adding the functionality described in ticket :ticket:`24788` to Django. -Writing the code for ticket #17549 +Writing the code for ticket #24788 ---------------------------------- -Navigate to the ``django/django/contrib/admin/`` folder and open the -``widgets.py`` file. Find the ``AdminURLFieldWidget`` class on line 302 and add -the following ``render`` method after the existing ``__init__`` method:: +Navigate to the ``django/django/forms/`` folder and open the ``forms.py`` file. +Find the ``BaseForm`` class on line 72 and add the ``prefix`` class attribute +right after the ``field_order`` attribute:: - def render(self, name, value, attrs=None): - html = super(AdminURLFieldWidget, self).render(name, value, attrs) - if value: - value = force_text(self._format_value(value)) - final_attrs = {'href': mark_safe(smart_urlquote(value))} - html = format_html( - '

{} {}
{} {}

', - _('Currently:'), flatatt(final_attrs), value, - _('Change:'), html - ) - return html + class BaseForm(object): + # This is the main implementation of all the Form logic. Note that this + # class is different than Form. See the comments by the Form class for more + # information. Any improvements to the form API should be made to *this* + # class, not to the Form class. + field_order = None + prefix = None Verifying your test now passes ------------------------------ Once you're done modifying Django, we need to make sure that the tests we wrote earlier pass, so we can see whether the code we wrote above is working -correctly. To run the tests in the ``admin_widgets`` folder, ``cd`` into the -Django ``tests/`` directory and run:: +correctly. To run the tests in the ``forms_tests`` folder, ``cd`` into the +Django ``tests/`` directory and run: - PYTHONPATH=.. python runtests.py --settings=test_sqlite admin_widgets +.. code-block:: console -Oops, good thing we wrote those tests! You should still see 3 failures with + $ PYTHONPATH=.. ./runtests.py forms_tests + +Oops, good thing we wrote those tests! You should still see one failure with the following exception:: - NameError: global name 'smart_urlquote' is not defined + AssertionError: None != 'foo' -We forgot to add the import for that method. Go ahead and add the -``smart_urlquote`` import at the end of line 13 of -``django/contrib/admin/widgets.py`` so it looks as follows:: +We forgot to add the conditional statement in the ``__init__`` method. Go ahead +and change ``self.prefix = prefix`` that is now on line 87 of +``django/forms/forms.py``, adding a conditional statement:: - from django.utils.html import escape, format_html, format_html_join, smart_urlquote + if prefix is not None: + self.prefix = prefix Re-run the tests and everything should pass. If it doesn't, make sure you -correctly modified the ``AdminURLFieldWidget`` class as shown above and -copied the new tests correctly. +correctly modified the ``BaseForm`` class as shown above and copied the new test +correctly. Running Django's test suite for the second time =============================================== @@ -377,27 +376,36 @@ passing the entire test suite doesn't guarantee your code is bug free, it does help identify many bugs and regressions that might otherwise go unnoticed. To run the entire Django test suite, ``cd`` into the Django ``tests/`` -directory and run:: +directory and run: - PYTHONPATH=.. python runtests.py --settings=test_sqlite +.. code-block:: console -As long as you don't see any failures, you're good to go. Note that this fix -also made a `small CSS change`__ to format the new widget. You can make the -change if you'd like, but we'll skip it for now in the interest of brevity. + $ PYTHONPATH=.. ./runtests.py -__ https://github.com/django/django/commit/ac2052ebc84c45709ab5f0f25e685bf656ce79bc#diff-0 +As long as you don't see any failures, you're good to go. Writing Documentation ===================== -This is a new feature, so it should be documented. Add the following on line -925 of ``django/docs/ref/models/fields.txt`` beneath the existing docs for -``URLField``:: +This is a new feature, so it should be documented. Add the following section on +line 1068 (at the end of the file) of ``django/docs/ref/forms/api.txt``:: - .. versionadded:: 1.5 + The prefix can also be specified on the form class:: - The current value of the field will be displayed as a clickable link above the - input widget. + >>> class PersonForm(forms.Form): + ... ... + ... prefix = 'person' + + .. versionadded:: 1.9 + + The ability to specify ``prefix`` on the form class was added. + +Since this new feature will be in an upcoming release it is also added to the +release notes for Django 1.9, on line 164 under the "Forms" section in the file +``docs/releases/1.9.txt``:: + + * A form prefix can be specified inside a form class, not only when + instantiating a form. See :ref:`form-prefix` for details. For more information on writing documentation, including an explanation of what the ``versionadded`` bit is all about, see @@ -410,113 +418,106 @@ Generating a patch for your changes Now it's time to generate a patch file that can be uploaded to Trac or applied to another copy of Django. To get a look at the content of your patch, run the -following command:: +following command: - git diff +.. code-block:: console + + $ git diff This will display the differences between your current copy of Django (with your changes) and the revision that you initially checked out earlier in the tutorial. Once you're done looking at the patch, hit the ``q`` key to exit back to the -command line. If the patch's content looked okay, you can run the following -command to save the patch file to your current working directory:: +command line. If the patch's content looked okay, you can run the following +command to save the patch file to your current working directory: - git diff > 17549.diff +.. code-block:: console -You should now have a file in the root Django directory called ``17549.diff``. + $ git diff > 24788.diff + +You should now have a file in the root Django directory called ``24788.diff``. This patch file contains all your changes and should look this: .. code-block:: diff - diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py - index 1e0bc2d..9e43a10 100644 - --- a/django/contrib/admin/widgets.py - +++ b/django/contrib/admin/widgets.py - @@ -10,7 +10,7 @@ from django.contrib.admin.templatetags.admin_static import static - from django.core.urlresolvers import reverse - from django.forms.widgets import RadioFieldRenderer - from django.forms.util import flatatt - -from django.utils.html import escape, format_html, format_html_join - +from django.utils.html import escape, format_html, format_html_join, smart_urlquote - from django.utils.text import Truncator - from django.utils.translation import ugettext as _ - from django.utils.safestring import mark_safe - @@ -306,6 +306,18 @@ class AdminURLFieldWidget(forms.TextInput): - final_attrs.update(attrs) - super(AdminURLFieldWidget, self).__init__(attrs=final_attrs) + diff --git a/django/forms/forms.py b/django/forms/forms.py + index 509709f..d1370de 100644 + --- a/django/forms/forms.py + +++ b/django/forms/forms.py + @@ -75,6 +75,7 @@ class BaseForm(object): + # information. Any improvements to the form API should be made to *this* + # class, not to the Form class. + field_order = None + + prefix = None - + def render(self, name, value, attrs=None): - + html = super(AdminURLFieldWidget, self).render(name, value, attrs) - + if value: - + value = force_text(self._format_value(value)) - + final_attrs = {'href': mark_safe(smart_urlquote(value))} - + html = format_html( - + '

{} {}
{} {}

', - + _('Currently:'), flatatt(final_attrs), value, - + _('Change:'), html - + ) - + return html + def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None, + initial=None, error_class=ErrorList, label_suffix=None, + @@ -83,7 +84,8 @@ class BaseForm(object): + self.data = data or {} + self.files = files or {} + self.auto_id = auto_id + - self.prefix = prefix + + if prefix is not None: + + self.prefix = prefix + self.initial = initial or {} + self.error_class = error_class + # Translators: This is the default suffix added to form field labels + diff --git a/docs/ref/forms/api.txt b/docs/ref/forms/api.txt + index 3bc39cd..008170d 100644 + --- a/docs/ref/forms/api.txt + +++ b/docs/ref/forms/api.txt + @@ -1065,3 +1065,13 @@ You can put several Django forms inside one ``
`` tag. To give each + >>> print(father.as_ul()) +
    • +
    • + - class AdminIntegerFieldWidget(forms.TextInput): - class_name = 'vIntegerField' - - diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt - index 809d56e..d44f85f 100644 - --- a/docs/ref/models/fields.txt - +++ b/docs/ref/models/fields.txt - @@ -922,6 +922,10 @@ Like all :class:`CharField` subclasses, :class:`URLField` takes the optional - :attr:`~CharField.max_length`argument. If you don't specify - :attr:`~CharField.max_length`, a default of 200 is used. - - +.. versionadded:: 1.5 + +The prefix can also be specified on the form class:: + - +The current value of the field will be displayed as a clickable link above the - +input widget. - - Relationship fields - =================== - diff --git a/tests/regressiontests/admin_widgets/tests.py b/tests/regressiontests/admin_widgets/tests.py - index 4b11543..94acc6d 100644 - --- a/tests/regressiontests/admin_widgets/tests.py - +++ b/tests/regressiontests/admin_widgets/tests.py - - @@ -265,6 +265,35 @@ class AdminSplitDateTimeWidgetTest(DjangoTestCase): - '

    Datum:
    Zeit:

    ', - ) - - +class AdminURLWidgetTest(DjangoTestCase): - + def test_render(self): - + w = widgets.AdminURLFieldWidget() - + self.assertHTMLEqual( - + conditional_escape(w.render('test', '')), - + '' - + ) - + self.assertHTMLEqual( - + conditional_escape(w.render('test', 'http://example.com')), - + '

    Currently:http://example.com
    Change:

    ' - + ) + + >>> class PersonForm(forms.Form): + + ... ... + + ... prefix = 'person' + - + def test_render_idn(self): - + w = widgets.AdminURLFieldWidget() - + self.assertHTMLEqual( - + conditional_escape(w.render('test', 'http://example-äüö.com')), - + '

    Currently:http://example-äüö.com
    Change:

    ' - + ) + +.. versionadded:: 1.9 + - + def test_render_quoting(self): - + w = widgets.AdminURLFieldWidget() - + self.assertHTMLEqual( - + conditional_escape(w.render('test', 'http://example.com/some text')), - + '

    Currently:http://example.com/<sometag>some text</sometag>
    Change:

    ' - + ) - + self.assertHTMLEqual( - + conditional_escape(w.render('test', 'http://example-äüö.com/some text')), - + '

    Currently:http://example-äüö.com/<sometag>some text</sometag>
    Change:

    ' - + ) + + The ability to specify ``prefix`` on the form class was added. + diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt + index 5b58f79..f9bb9de 100644 + --- a/docs/releases/1.9.txt + +++ b/docs/releases/1.9.txt + @@ -161,6 +161,9 @@ Forms + :attr:`~django.forms.Form.field_order` attribute, the ``field_order`` + constructor argument , or the :meth:`~django.forms.Form.order_fields` method. - class AdminFileWidgetTest(DjangoTestCase): - def test_render(self): + +* A form prefix can be specified inside a form class, not only when + + instantiating a form. See :ref:`form-prefix` for details. + + + Generic Views + ^^^^^^^^^^^^^ + + diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py + index 690f205..e07fae2 100644 + --- a/tests/forms_tests/tests/test_forms.py + +++ b/tests/forms_tests/tests/test_forms.py + @@ -1671,6 +1671,18 @@ class FormsTestCase(SimpleTestCase): + self.assertEqual(p.cleaned_data['last_name'], 'Lennon') + self.assertEqual(p.cleaned_data['birthday'], datetime.date(1940, 10, 9)) + + + def test_class_prefix(self): + + # Prefix can be also specified at the class level. + + class Person(Form): + + first_name = CharField() + + prefix = 'foo' + + + + p = Person() + + self.assertEqual(p.prefix, 'foo') + + + + p = Person(prefix='bar') + + self.assertEqual(p.prefix, 'bar') + + + def test_forms_with_null_boolean(self): + # NullBooleanField is a bit of a special case because its presentation (widget) + # is different than its data. This is handled transparently, though. So what do I do next? ===================== @@ -529,10 +530,12 @@ oriented workflow ` is recommended. Since we never committed our changes locally, perform the following to get your -git branch back to a good starting point:: +git branch back to a good starting point: - git reset --hard HEAD - git checkout master +.. code-block:: console + + $ git reset --hard HEAD + $ git checkout master More information for new contributors ------------------------------------- @@ -561,7 +564,7 @@ Finding your first real ticket Once you've looked through some of that information, you'll be ready to go out and find a ticket of your own to write a patch for. Pay special attention to tickets with the "easy pickings" criterion. These tickets are often much -simpler in nature and are great for first time contributors. Once you're +simpler in nature and are great for first time contributors. Once you're familiar with contributing to Django, you can move on to writing patches for more difficult and complicated tickets. diff --git a/docs/spelling_wordlist b/docs/spelling_wordlist index 0fe8fb7886..d175711489 100644 --- a/docs/spelling_wordlist +++ b/docs/spelling_wordlist @@ -455,6 +455,7 @@ makemessages makemigrations Mako Mapnik +Marczewski Marino Markus MBR @@ -545,6 +546,7 @@ Palau parameterized params parens +Paweł pdf PEM perl