Merge pull request #122 from wagtail/120-improve-inline-comments

Improve inline comments

bakerydemo/base/models.py

@@ -6,7 +6,12 @@ from modelcluster.fields import ParentalKey
 from modelcluster.models import ClusterableModel
 
 from wagtail.wagtailadmin.edit_handlers import (
-    FieldPanel, FieldRowPanel, InlinePanel, MultiFieldPanel, PageChooserPanel, StreamFieldPanel,
+    FieldPanel,
+    FieldRowPanel,
+    InlinePanel,
+    MultiFieldPanel,
+    PageChooserPanel,
+    StreamFieldPanel,
 )
 from wagtail.wagtailcore.fields import RichTextField, StreamField
 from wagtail.wagtailcore.models import Collection, Page
@@ -21,8 +26,16 @@ from .blocks import BaseStreamBlock
 @register_snippet
 class People(ClusterableModel):
     """
-    `People` snippets are secondary content objects that do not require their
-    own full webpage to render.
+    A Django model to store People objects.
+    It uses the `@register_snippet` decorator to allow it to be accessible
+    via the Snippets UI (e.g. /admin/snippets/base/people/)
+
+    `People` uses the `ClusterableModel`, which allows the relationship with
+    another model to be stored locally to the 'parent' model (e.g. a PageModel)
+    until the parent is explicitly saved. This allows the editor to use the
+    'Preview' button, to preview the content, without saving the relationships
+    to the database.
+    https://github.com/wagtail/django-modelcluster
     """
     first_name = models.CharField("First name", max_length=254)
     last_name = models.CharField("Last name", max_length=254)
@@ -50,8 +63,8 @@ class People(ClusterableModel):
 
     @property
     def thumb_image(self):
-        # fail silently if there is no profile pic or the rendition file can't
-        # be found. Note @richbrennan worked out how to do this...
+        # Returns an empty string if there is no profile pic or the rendition
+        # file can't be found.
         try:
             return self.image.get_rendition('fill-50x50').img_tag()
         except:
@@ -68,7 +81,10 @@ class People(ClusterableModel):
 @register_snippet
 class FooterText(models.Model):
     """
-    This provides editable text for the site footer
+    This provides editable text for the site footer. Again it uses the decorator
+    `register_snippet` to allow it to be accessible via the admin. It is made
+    accessible on the template via a template tag defined in base/templatetags/
+    navigation_tags.py
     """
     body = RichTextField()
 
@@ -85,7 +101,9 @@ class FooterText(models.Model):
 
 class StandardPage(Page):
     """
-    A fairly generic site page, to be used for About, etc.
+    A generic content page. On this demo site we use it for an about page but
+    it could be used for any type of page content that only needs a title,
+    image, introduction and body field
     """
 
     introduction = models.TextField(
@@ -111,8 +129,16 @@ class StandardPage(Page):
 
 class HomePage(Page):
     """
-    The Home Page
+    The Home Page. This looks slightly more complicated than it is. You can
+    see if you visit your site and edit the homepage that it is split between
+    a:
+    - Hero area
+    - Body area
+    - A promotional area
+    - Moveable featured site sections
     """
+
+    # Hero section of HomePage
     image = models.ForeignKey(
         'wagtailimages.Image',
         null=True,
@@ -140,10 +166,12 @@ class HomePage(Page):
         help_text='Choose a page to link to for the Call to Action'
     )
 
+    # Body section of the HomePage
     body = StreamField(
         BaseStreamBlock(), verbose_name="Home content block", blank=True
     )
 
+    # Promo section of the HomePage
     promo_image = models.ForeignKey(
         'wagtailimages.Image',
         null=True,
@@ -164,6 +192,11 @@ class HomePage(Page):
         help_text='Write some promotional copy'
     )
 
+    # Featured sections on the HomePage
+    # You will see on templates/base/home_page.html that these are treated
+    # in different ways, and displayed in different areas of the page.
+    # Each list their children items that we access via the children function
+    # that we define on the individual Page models e.g. BlogIndexPage
     featured_section_1_title = models.CharField(
         null=True,
         blank=True,
@@ -176,7 +209,8 @@ class HomePage(Page):
         blank=True,
         on_delete=models.SET_NULL,
         related_name='+',
-        help_text='First featured section for the homepage. Will display up to three child items.',
+        help_text='First featured section for the homepage. Will display up to '
+        'three child items.',
         verbose_name='Featured section 1'
     )
 
@@ -192,7 +226,8 @@ class HomePage(Page):
         blank=True,
         on_delete=models.SET_NULL,
         related_name='+',
-        help_text='Second featured section for the homepage. Will display up to three child items.',
+        help_text='Second featured section for the homepage. Will display up to '
+        'three child items.',
         verbose_name='Featured section 2'
     )
 
@@ -208,7 +243,8 @@ class HomePage(Page):
         blank=True,
         on_delete=models.SET_NULL,
         related_name='+',
-        help_text='Third featured section for the homepage. Will display up to six child items.',
+        help_text='Third featured section for the homepage. Will display up to '
+        'six child items.',
         verbose_name='Featured section 3'
     )
 
@@ -249,7 +285,10 @@ class HomePage(Page):
 
 class GalleryPage(Page):
     """
-    This is a page to list locations from the selected Collection
+    This is a page to list locations from the selected Collection. We use a Q
+    object to list any Collection created (/admin/collections/) even if they
+    contain no items. In this demo we use it for a GalleryPage,
+    and is intended to show the extensibility of this aspect of Wagtail
     """
 
     introduction = models.TextField(
@@ -261,7 +300,8 @@ class GalleryPage(Page):
         blank=True,
         on_delete=models.SET_NULL,
         related_name='+',
-        help_text='Landscape mode only; horizontal width between 1000px and 3000px.'
+        help_text='Landscape mode only; horizontal width between 1000px and '
+        '3000px.'
     )
     body = StreamField(
         BaseStreamBlock(), verbose_name="Page body", blank=True
@@ -288,6 +328,14 @@ class GalleryPage(Page):
 
 
 class FormField(AbstractFormField):
+    """
+    Wagtailforms is a module to introduce simple forms on a Wagtail site. It
+    isn't intended as a replacement to Django's form support but as a quick way
+    to generate a general purpose data-collection form or contact form
+    without having to write code. We use it on the site for a contact form. You
+    can read more about Wagtail forms at:
+    http://docs.wagtail.io/en/latest/reference/contrib/forms/index.html
+    """
     page = ParentalKey('FormPage', related_name='form_fields')
 
 
@@ -301,6 +349,9 @@ class FormPage(AbstractEmailForm):
     )
     body = StreamField(BaseStreamBlock())
     thank_you_text = RichTextField(blank=True)
+
+    # Note how we include the FormField object via an InlinePanel using the
+    # related_name value
     content_panels = AbstractEmailForm.content_panels + [
         ImageChooserPanel('image'),
         StreamFieldPanel('body'),

bakerydemo/blog/models.py

@@ -23,7 +23,10 @@ from bakerydemo.base.blocks import BaseStreamBlock
 class BlogPeopleRelationship(Orderable, models.Model):
     """
     This defines the relationship between the `People` within the `base`
-    app and the BlogPage below allowing us to add people to a BlogPage.
+    app and the BlogPage below. This allows People to be added to a BlogPage.
+
+    We have created a two way relationship between BlogPage and People using
+    the ParentalKey and ForeignKey
     """
     page = ParentalKey(
         'BlogPage', related_name='blog_person_relationship'
@@ -37,12 +40,21 @@ class BlogPeopleRelationship(Orderable, models.Model):
 
 
 class BlogPageTag(TaggedItemBase):
+    """
+    This model allows us to create a many-to-many relationship between
+    the BlogPage object and tags. There's a longer guide on using it at
+    http://docs.wagtail.io/en/latest/reference/pages/model_recipes.html#tagging
+    """
     content_object = ParentalKey('BlogPage', related_name='tagged_items')
 
 
 class BlogPage(Page):
     """
-    A Blog Page (Post)
+    A Blog Page
+
+    We access the People object with an inline panel that references the
+    ParentalKey's related_name in BlogPeopleRelationship. More docs:
+    http://docs.wagtail.io/en/latest/topics/pages.html#inline-models
     """
     introduction = models.TextField(
         help_text='Text to describe the page',
@@ -60,7 +72,9 @@ class BlogPage(Page):
     )
     subtitle = models.CharField(blank=True, max_length=255)
     tags = ClusterTaggableManager(through=BlogPageTag, blank=True)
-    date_published = models.DateField("Date article published", blank=True, null=True)
+    date_published = models.DateField(
+        "Date article published", blank=True, null=True
+        )
 
     content_panels = Page.content_panels + [
         FieldPanel('subtitle', classname="full"),
@@ -81,7 +95,11 @@ class BlogPage(Page):
 
     def authors(self):
         """
-        Returns the BlogPage's related People
+        Returns the BlogPage's related People. Again note that we are using
+        the ParentalKey's related_name from the BlogPeopleRelationship model
+        to access these objects. This allows us to access the People objects
+        with a loop on the template. If we tried to access the blog_person_
+        relationship directly we'd print `blog.BlogPeopleRelationship.None`
         """
         authors = [
             n.people for n in self.blog_person_relationship.all()
@@ -92,8 +110,9 @@ class BlogPage(Page):
     @property
     def get_tags(self):
         """
-        Returns the BlogPage's related list of Tags.
-        Each Tag is modified to include a url attribute
+        Similar to the authors function above we're returning all the tags that
+        are related to the blog post into a list we can access on the template.
+        We're additionally adding a URL to access BlogPage objects with that tag
         """
         tags = self.tags.all()
         for tag in tags:
@@ -104,9 +123,10 @@ class BlogPage(Page):
             ])
         return tags
 
+    # Specifies parent to BlogPage as being BlogIndexPages
     parent_page_types = ['BlogIndexPage']
 
-    # Define what content types can exist as children of BlogPage.
+    # Specifies what content types can exist as children of BlogPage.
     # Empty list means that no child content types are allowed.
     subpage_types = []
 
@@ -114,12 +134,12 @@ class BlogPage(Page):
 class BlogIndexPage(RoutablePageMixin, Page):
     """
     Index page for blogs.
-    We need to alter the page model's context to return the child page objects - the
-    BlogPage - so that it works as an index page
+    We need to alter the page model's context to return the child page objects,
+    the BlogPage objects, so that it works as an index page
 
-    RoutablePageMixin is used to allow for a custom sub-URL for tag views.
+    RoutablePageMixin is used to allow for a custom sub-URL for the tag views
+    defined above.
     """
-
     introduction = models.TextField(
         help_text='Text to describe the page',
         blank=True)
@@ -132,12 +152,22 @@ class BlogIndexPage(RoutablePageMixin, Page):
         help_text='Landscape mode only; horizontal width between 1000px and 3000px.'
     )
 
-    # What pages types can live under this page type?
+    content_panels = Page.content_panels + [
+        FieldPanel('introduction', classname="full"),
+        ImageChooserPanel('image'),
+    ]
+
+    # Speficies that only BlogPage objects can live under this index page
     subpage_types = ['BlogPage']
 
+    # Defines a method to access the children of the page (e.g. BlogPage
+    # objects). On the demo site we use this on the HomePage
     def children(self):
         return self.get_children().specific().live()
 
+    # Overrides the context to list all child items, that are live, by the
+    # date that they were published
+    # http://docs.wagtail.io/en/latest/getting_started/tutorial.html#overriding-context
     def get_context(self, request):
         context = super(BlogIndexPage, self).get_context(request)
         context['posts'] = BlogPage.objects.descendant_of(
@@ -145,14 +175,13 @@ class BlogIndexPage(RoutablePageMixin, Page):
             '-date_published')
         return context
 
+    # This defines a Custom view that utilizes Tags. This view will return all
+    # related BlogPages for a given Tag or redirect back to the BlogIndexPage.
+    # More information on RoutablePages is at
+    # http://docs.wagtail.io/en/latest/reference/contrib/routablepage.html
     @route('^tags/$', name='tag_archive')
     @route('^tags/(\w+)/$', name='tag_archive')
     def tag_archive(self, request, tag=None):
-        """
-        A Custom view that utilizes Tags.
-        This view will return all related BlogPages for a given Tag or
-        redirect back to the BlogIndexPage.
-        """
 
         try:
             tag = Tag.objects.get(slug=tag)
@@ -163,36 +192,25 @@ class BlogIndexPage(RoutablePageMixin, Page):
             return redirect(self.url)
 
         posts = self.get_posts(tag=tag)
-
         context = {
             'tag': tag,
             'posts': posts
         }
         return render(request, 'blog/blog_index_page.html', context)
 
+    # Returns the child BlogPage objects for this BlogPageIndex.
+    # If a tag is used then it will filter the posts by tag.
     def get_posts(self, tag=None):
-        """
-        Return the child BlogPage objects for this BlogPageIndex.
-        Optional filter by tag.
-        """
-
         posts = BlogPage.objects.live().descendant_of(self)
         if tag:
             posts = posts.filter(tags=tag)
         return posts
 
+    # Returns the list of Tags for all child posts of this BlogPage.
     def get_child_tags(self):
-        """
-        Returns the list of Tags for all child posts of this BlogPage.
-        """
-
         tags = []
         for post in self.get_posts():
-            tags += post.get_tags  # Not tags.append() because we don't want a list of lists
+            # Not tags.append() because we don't want a list of lists
+            tags += post.get_tags
         tags = sorted(set(tags))
         return tags
-
-    content_panels = Page.content_panels + [
-        FieldPanel('introduction', classname="full"),
-        ImageChooserPanel('image'),
-    ]

bakerydemo/breads/models.py

@@ -4,7 +4,9 @@ from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
 
 from modelcluster.fields import ParentalManyToManyField
 
-from wagtail.wagtailadmin.edit_handlers import FieldPanel, MultiFieldPanel, StreamFieldPanel
+from wagtail.wagtailadmin.edit_handlers import (
+    FieldPanel, MultiFieldPanel, StreamFieldPanel
+    )
 from wagtail.wagtailcore.fields import StreamField
 from wagtail.wagtailcore.models import Page
 from wagtail.wagtailsearch import index
@@ -17,8 +19,13 @@ from bakerydemo.base.blocks import BaseStreamBlock
 @register_snippet
 class Country(models.Model):
     """
-    Standard Django model to store set of countries of origin.
-    Exposed in the Wagtail admin via Snippets.
+    A Django model to store set of countries of origin.
+    It uses the `@register_snippet` decorator to allow it to be accessible
+    via the Snippets UI (e.g. /admin/snippets/breads/country/) In the BreadPage
+    model you'll see we use a ForeignKey to create the relationship between
+    Country and BreadPage. This allows a single relationship (e.g only one
+    Country can be added) that is one-way (e.g. Country will have no way to
+    access related BreadPage objects).
     """
 
     title = models.CharField(max_length=100)
@@ -33,8 +40,11 @@ class Country(models.Model):
 @register_snippet
 class BreadIngredient(models.Model):
     """
-    Standard Django model used as a Snippet in the BreadPage model.
-    Demonstrates ManyToMany relationship.
+    Standard Django model that is displayed as a snippet within the admin due
+    to the `@register_snippet` decorator. We use a new piece of functionality
+    available to Wagtail called the ParentalManyToManyField on the BreadPage
+    model to display this. The Wagtail Docs give a slightly more detailed example
+    http://docs.wagtail.io/en/latest/getting_started/tutorial.html#categories
     """
     name = models.CharField(max_length=255)
 
@@ -52,7 +62,12 @@ class BreadIngredient(models.Model):
 @register_snippet
 class BreadType(models.Model):
     """
-    Standard Django model used as a Snippet in the BreadPage model.
+    A Django model to define the bread type
+    It uses the `@register_snippet` decorator to allow it to be accessible
+    via the Snippets UI. In the BreadPage model you'll see we use a ForeignKey
+    to create the relationship between BreadType and BreadPage. This allows a
+    single relationship (e.g only one BreadType can be added) that is one-way
+    (e.g. BreadType will have no way to access related BreadPage objects)
     """
 
     title = models.CharField(max_length=255)
@@ -92,6 +107,12 @@ class BreadPage(Page):
         null=True,
         blank=True,
     )
+
+    # We include related_name='+' to avoid name collisions on relationships.
+    # e.g. there are two FooPage models in two different apps,
+    # and they both have a FK to bread_type, they'll both try to create a
+    # relationship called `foopage_objects` that will throw a valueError on
+    # collision.
     bread_type = models.ForeignKey(
         'breads.BreadType',
         null=True,
@@ -129,9 +150,11 @@ class BreadPage(Page):
 
 class BreadsIndexPage(Page):
     """
-    Index page for breads. We don't have any fields within our model but we need
-    to alter the page model's context to return the child page objects - the
-    BreadPage - so that it works as an index page.
+    Index page for breads.
+
+    This is more complex than other index pages on the bakery demo site as we've
+    included pagination. We've separated the different aspects of the index page
+    to be discrete functions to make it easier to follow
     """
 
     introduction = models.TextField(
@@ -143,17 +166,33 @@ class BreadsIndexPage(Page):
         blank=True,
         on_delete=models.SET_NULL,
         related_name='+',
-        help_text='Landscape mode only; horizontal width between 1000px and 3000px.'
+        help_text='Landscape mode only; horizontal width between 1000px and '
+        '3000px.'
     )
+
+    content_panels = Page.content_panels + [
+        FieldPanel('introduction', classname="full"),
+        ImageChooserPanel('image'),
+    ]
+
+    # Can only have BreadPage children
     subpage_types = ['BreadPage']
 
+    # Returns a queryset of BreadPage objects that are live, that are direct
+    # descendants of this index page with most recent first
     def get_breads(self):
         return BreadPage.objects.live().descendant_of(
             self).order_by('-first_published_at')
 
+    # Allows child objects (e.g. BreadPage objects) to be accessible via the
+    # template. We use this on the HomePage to display child items of featured
+    # content
     def children(self):
         return self.get_children().specific().live()
 
+    # Pagination for the index page. We use the `django.core.paginator` as any
+    # standard Django app would, but the difference here being we have it as a
+    # method on the model rather than within a view function
     def paginate(self, request, *args):
         page = request.GET.get('page')
         paginator = Paginator(self.get_breads(), 12)
@@ -165,16 +204,14 @@ class BreadsIndexPage(Page):
             pages = paginator.page(paginator.num_pages)
         return pages
 
+    # Returns the above to the get_context method that is used to populate the
+    # template
     def get_context(self, request):
         context = super(BreadsIndexPage, self).get_context(request)
 
+        # BreadPage objects (get_breads) are passed through pagination
         breads = self.paginate(request, self.get_breads())
 
         context['breads'] = breads
 
         return context
-
-    content_panels = Page.content_panels + [
-        FieldPanel('introduction', classname="full"),
-        ImageChooserPanel('image'),
-    ]

bakerydemo/locations/models.py

@@ -18,7 +18,7 @@ from bakerydemo.locations.choices import DAY_CHOICES
 
 class OperatingHours(models.Model):
     """
-    Django model to capture operating hours for a Location
+    A Django model to capture operating hours for a Location
     """
 
     day = models.CharField(
@@ -28,10 +28,12 @@ class OperatingHours(models.Model):
     )
     opening_time = models.TimeField(
         blank=True,
-        null=True)
+        null=True
+    )
     closing_time = models.TimeField(
         blank=True,
-        null=True)
+        null=True
+    )
     closed = models.BooleanField(
         "Closed?",
         blank=True,
@@ -67,7 +69,12 @@ class OperatingHours(models.Model):
 
 class LocationOperatingHours(Orderable, OperatingHours):
     """
-    Operating Hours entry for a Location
+    A model creating a relationship between the OperatingHours and Location
+    Note that unlike BlogPeopleRelationship we don't include a ForeignKey to
+    OperatingHours as we don't need that relationship (e.g. any Location open
+    a certain day of the week). The ParentalKey is the minimum required to
+    relate the two objects to one another. We use the ParentalKey's related_
+    name to access it from the LocationPage admin
     """
     location = ParentalKey(
         'LocationPage',
@@ -77,9 +84,8 @@ class LocationOperatingHours(Orderable, OperatingHours):
 
 class LocationsIndexPage(Page):
     """
-    Index page for locations
+    A Page model that creates an index page (a listview)
     """
-
     introduction = models.TextField(
         help_text='Text to describe the page',
         blank=True)
@@ -92,11 +98,18 @@ class LocationsIndexPage(Page):
         help_text='Landscape mode only; horizontal width between 1000px and 3000px.'
     )
 
+    # Only LocationPage objects can be added underneath this index page
     subpage_types = ['LocationPage']
 
+    # Allows children of this indexpage to be accessible via the indexpage
+    # object on templates. We use this on the homepage to show featured
+    # sections of the site and their child pages
     def children(self):
         return self.get_children().specific().live()
 
+    # Overrides the context to list all child
+    # items, that are live, by the date that they were published
+    # http://docs.wagtail.io/en/latest/getting_started/tutorial.html#overriding-context
     def get_context(self, request):
         context = super(LocationsIndexPage, self).get_context(request)
         context['locations'] = LocationPage.objects.descendant_of(
@@ -148,7 +161,7 @@ class LocationPage(Page):
         index.SearchField('body'),
     ]
 
-    # Editor panels configuration
+    # Fields to show to the editor in the admin view
     content_panels = [
         FieldPanel('title', classname="full"),
         FieldPanel('introduction', classname="full"),
@@ -167,8 +180,8 @@ class LocationPage(Page):
         hours = self.hours_of_operation.all()
         return hours
 
+    # Determines if the location is currently open. It is timezone naive
     def is_open(self):
-        # Determines if the location is currently open
         now = datetime.now()
         current_time = now.time()
         current_day = now.strftime('%a').upper()
@@ -182,6 +195,8 @@ class LocationPage(Page):
         except LocationOperatingHours.DoesNotExist:
             return False
 
+    # Makes additional context available to the template so that we can access
+    # the latitude, longitude and map API key to render the map
     def get_context(self, request):
         context = super(LocationPage, self).get_context(request)
         context['lat'] = self.lat_long.split(",")[0]
@@ -189,4 +204,5 @@ class LocationPage(Page):
         context['google_map_api_key'] = settings.GOOGLE_MAP_API_KEY
         return context
 
+    # Can only be placed under a LocationsIndexPage object
     parent_page_types = ['LocationsIndexPage']