From d778c3693d152d9c4965b3a02597c6494b077e81 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Thu, 19 Jun 2014 12:17:07 +0100 Subject: [PATCH 01/72] respect DOM re-ordering of formsets even if the parent form errored --- wagtail/wagtailadmin/edit_handlers.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/wagtail/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index a15440016..d7a5726c3 100644 --- a/wagtail/wagtailadmin/edit_handlers.py +++ b/wagtail/wagtailadmin/edit_handlers.py @@ -562,6 +562,11 @@ class BaseInlinePanel(EditHandler): child_edit_handler_class(instance=subform.instance, form=subform) ) + # if this formset is valid, it may have been re-ordered; respect that + # in case the parent form errored and we need to re-render + if self.formset.can_order and self.formset.is_valid(): + self.children = sorted(self.children, key=lambda x: x.form.cleaned_data['ORDER']) + empty_form = self.formset.empty_form empty_form.fields['DELETE'].widget = forms.HiddenInput() if self.formset.can_order: From 5c64172d02fc2fded5f4ed990f62fa836c5302c3 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 13:44:54 +0100 Subject: [PATCH 02/72] Created ElasticSearchMapping class --- .../wagtailsearch/backends/elasticsearch.py | 92 ++++++++++++------- 1 file changed, 59 insertions(+), 33 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 91536d744..82e76d5fc 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -12,6 +12,40 @@ from wagtail.wagtailsearch.indexed import Indexed from wagtail.wagtailsearch.utils import normalise_query_string +class ElasticSearchMapping(object): + def __init__(self, model): + self.model = model + + def get_document_type(self): + return self.model.indexed_get_content_type() + + def get_mapping(self): + # Get type name + content_type = self.get_document_type() + + # Get indexed fields + indexed_fields = self.model.indexed_get_indexed_fields() + + # Make field list + fields = { + 'pk': dict(type='string', index='not_analyzed', store='yes'), + 'content_type': dict(type='string'), + } + fields.update(indexed_fields) + + return { + content_type: { + 'properties': fields, + } + } + + def get_document_id(self, obj): + return obj.indexed_build_document()['id'] + + def get_document(self, obj): + return obj.indexed_build_document() + + class ElasticSearchQuery(object): def __init__(self, model, query_string, fields=None, filters={}): self.model = model @@ -330,25 +364,11 @@ class ElasticSearch(BaseSearch): self.es.indices.create(self.es_index, INDEX_SETTINGS) def add_type(self, model): - # Get type name - content_type = model.indexed_get_content_type() - - # Get indexed fields - indexed_fields = model.indexed_get_indexed_fields() - - # Make field list - fields = { - "pk": dict(type="string", index="not_analyzed", store="yes"), - "content_type": dict(type="string"), - } - fields.update(indexed_fields) + # Get mapping + mapping = ElasticSearchMapping(model) # Put mapping - self.es.indices.put_mapping(index=self.es_index, doc_type=content_type, body={ - content_type: { - "properties": fields, - } - }) + self.es.indices.put_mapping(index=self.es_index, doc_type=mapping.get_document_type(), body=mapping.get_mapping()) def refresh_index(self): self.es.indices.refresh(self.es_index) @@ -358,11 +378,11 @@ class ElasticSearch(BaseSearch): if not self.object_can_be_indexed(obj): return - # Build document - doc = obj.indexed_build_document() + # Get mapping + mapping = ElasticSearchMapping(obj.__class__) - # Add to index - self.es.index(self.es_index, obj.indexed_get_content_type(), doc, id=doc["id"]) + # Add document to index + self.es.index(self.es_index, mapping.get_document_type(), mapping.get_document(obj), id=mapping.get_document_id(obj)) def add_bulk(self, obj_list): # Group all objects by their type @@ -372,27 +392,30 @@ class ElasticSearch(BaseSearch): if not self.object_can_be_indexed(obj): continue - # Get object type - obj_type = obj.indexed_get_content_type() + # Get mapping + mapping = ElasticSearchMapping(obj.__class__) + + # Get document type + doc_type = mapping.get_document_type() # If type is currently not in set, add it - if obj_type not in type_set: - type_set[obj_type] = [] + if doc_type not in type_set: + type_set[doc_type] = [] - # Add object to set - type_set[obj_type].append(obj.indexed_build_document()) + # Add document to set + type_set[doc_type].append((mapping.get_document_id(obj), mapping.get_document(obj))) # Loop through each type and bulk add them - for type_name, type_objects in type_set.items(): + for type_name, type_documents in type_set.items(): # Get list of actions actions = [] - for obj in type_objects: + for doc_id, doc in type_documents: action = { '_index': self.es_index, '_type': type_name, - '_id': obj['id'], + '_id': doc_id, } - action.update(obj) + action.update(doc) actions.append(action) bulk(self.es, actions) @@ -402,12 +425,15 @@ class ElasticSearch(BaseSearch): if not isinstance(obj, Indexed) or not isinstance(obj, models.Model): return + # Get mapping + mapping = ElasticSearchMapping(obj.__class__) + # Delete document try: self.es.delete( self.es_index, - obj.indexed_get_content_type(), - obj.indexed_get_document_id(), + mapping.get_document_type(), + mapping.get_document_id(obj), ) except NotFoundError: pass # Document doesn't exist, ignore this exception From fefa8c79ef7660d789f109a444ebe2b9fc8b43f8 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 13:51:35 +0100 Subject: [PATCH 03/72] Added repr to ElasticSearchMapping --- wagtail/wagtailsearch/backends/elasticsearch.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 82e76d5fc..9ad1f288a 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -45,6 +45,9 @@ class ElasticSearchMapping(object): def get_document(self, obj): return obj.indexed_build_document() + def __repr__(self): + return '' % (self.model.__name__, ) + class ElasticSearchQuery(object): def __init__(self, model, query_string, fields=None, filters={}): From da9b7c2408474583d66bf1be274e9a0831f36a40 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 13:55:09 +0100 Subject: [PATCH 04/72] Moved document building methods from indexed into ElasticSearchMapping --- .../wagtailsearch/backends/elasticsearch.py | 19 ++++++++++++++-- wagtail/wagtailsearch/indexed.py | 22 ------------------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 9ad1f288a..bbdbf0ecd 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -40,10 +40,25 @@ class ElasticSearchMapping(object): } def get_document_id(self, obj): - return obj.indexed_build_document()['id'] + return obj.indexed_get_toplevel_content_type() + ':' + str(obj.pk) def get_document(self, obj): - return obj.indexed_build_document() + # Get content type, indexed fields and id + content_type = obj.indexed_get_content_type() + indexed_fields = obj.indexed_get_indexed_fields() + + # Build document + doc = dict(pk=str(obj.pk), content_type=content_type) + for field in indexed_fields.keys(): + if hasattr(obj, field): + doc[field] = getattr(obj, field) + + # Check if this field is callable + if hasattr(doc[field], "__call__"): + # Call it + doc[field] = doc[field]() + + return doc def __repr__(self): return '' % (self.model.__name__, ) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index e2817905d..929bd0dbe 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -65,28 +65,6 @@ class Indexed(object): indexed_fields = parent_indexed_fields return indexed_fields - def indexed_get_document_id(self): - return self.indexed_get_toplevel_content_type() + ":" + str(self.pk) - - def indexed_build_document(self): - # Get content type, indexed fields and id - content_type = self.indexed_get_content_type() - indexed_fields = self.indexed_get_indexed_fields() - doc_id = self.indexed_get_document_id() - - # Build document - doc = dict(pk=str(self.pk), content_type=content_type, id=doc_id) - for field in indexed_fields.keys(): - if hasattr(self, field): - doc[field] = getattr(self, field) - - # Check if this field is callable - if hasattr(doc[field], "__call__"): - # Call it - doc[field] = doc[field]() - - return doc - indexed_fields = () From e81a9f6c6b3aed2a78ea16890dd34930b1f85ab9 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 14:56:56 +0100 Subject: [PATCH 05/72] Added repr to indexed.BaseField --- wagtail/wagtailsearch/indexed.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index 929bd0dbe..334bb30da 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -94,6 +94,9 @@ class BaseField(object): return dic + def __repr__(self): + return "<%s: %s>" % (self.__class__.__name__, self.field_name) + class SearchField(BaseField): def __init__(self, field_name, boost=None, partial_match=False, **kwargs): From 3c5a065a3601ff5b23df8ac9081dc5c88a532234 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:00:57 +0100 Subject: [PATCH 06/72] SearchFields configuration is now used in the backends Previously, we just converted them to make them look like indexed_fields. We now convert indexed_fields into SearchFields objects and pass them to the backend. --- wagtail/wagtailsearch/backends/db.py | 2 +- .../wagtailsearch/backends/elasticsearch.py | 20 ++----- wagtail/wagtailsearch/indexed.py | 56 +++++++++++++++++-- 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/wagtail/wagtailsearch/backends/db.py b/wagtail/wagtailsearch/backends/db.py index 214c71cb3..a7dca12e9 100644 --- a/wagtail/wagtailsearch/backends/db.py +++ b/wagtail/wagtailsearch/backends/db.py @@ -38,7 +38,7 @@ class DBSearch(BaseSearch): # Get fields if fields is None: - fields = model.indexed_get_indexed_fields().keys() + fields = [field.field_name for field in model.get_searchable_search_fields()] # Start will all objects query = model.objects.all() diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index bbdbf0ecd..28127fe83 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -20,21 +20,17 @@ class ElasticSearchMapping(object): return self.model.indexed_get_content_type() def get_mapping(self): - # Get type name - content_type = self.get_document_type() - - # Get indexed fields - indexed_fields = self.model.indexed_get_indexed_fields() - # Make field list fields = { 'pk': dict(type='string', index='not_analyzed', store='yes'), 'content_type': dict(type='string'), } - fields.update(indexed_fields) + + for field in self.model.get_search_fields(): + fields[field.get_attname(self.model)] = field.to_dict(self.model) return { - content_type: { + self.get_document_type(): { 'properties': fields, } } @@ -43,13 +39,9 @@ class ElasticSearchMapping(object): return obj.indexed_get_toplevel_content_type() + ':' + str(obj.pk) def get_document(self, obj): - # Get content type, indexed fields and id - content_type = obj.indexed_get_content_type() - indexed_fields = obj.indexed_get_indexed_fields() - # Build document - doc = dict(pk=str(obj.pk), content_type=content_type) - for field in indexed_fields.keys(): + doc = dict(pk=str(obj.pk), content_type=self.model.indexed_get_content_type()) + for field in [field.get_attname(self.model) for field in self.model.get_search_fields()]: if hasattr(obj, field): doc[field] = getattr(obj, field) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index 334bb30da..b4e67f4c1 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -1,3 +1,5 @@ +import warnings + from six import string_types from django.db import models @@ -35,11 +37,6 @@ class Indexed(object): @classmethod def indexed_get_indexed_fields(cls): - # New way - if hasattr(cls, 'search_fields'): - return dict((field.get_attname(cls), field.to_dict(cls)) for field in cls.search_fields) - - # Old way # Get indexed fields for this class as dictionary indexed_fields = cls.indexed_fields if isinstance(indexed_fields, dict): @@ -65,10 +62,57 @@ class Indexed(object): indexed_fields = parent_indexed_fields return indexed_fields + @classmethod + def get_search_fields(cls): + search_fields = [] + + if hasattr(cls, 'search_fields'): + search_fields.extend(cls.search_fields) + + # Backwards compatibility with old indexed_fields setting + + # Get indexed fields + indexed_fields = cls.indexed_get_indexed_fields() + + # Display deprecation warning if indexed_fields has been used + if indexed_fields: + warnings.warn("'indexed_fields' setting is now deprecated." + "Use 'search_fields' instead.", DeprecationWarning) + + # Convert them into search fields + for field_name, _config in indexed_fields.items(): + # Copy the config to prevent is trashing anything accidentally + config = _config.copy() + + # Check if this is a filter field + if config.get('index', None) == 'not_analyzed': + config.pop('index') + search_fields.append(FilterField(field_name, es_extra=config)) + continue + + # Must be a search field, check for boosting and partial matching + boost = config.pop('boost', None) + + partial_match = False + if config.get('analyzer', None) == 'edgengram_analyzer': + partial_match = True + config.pop('analyzer') + + # Add the field + search_fields.append(SearchField(field_name, boost=boost, partial_match=partial_match, es_extra=config)) + + return search_fields + + @classmethod + def get_searchable_search_fields(cls): + return filter(lambda field: field.searchable, cls.get_search_fields()) + indexed_fields = () class BaseField(object): + searchable = False + def __init__(self, field_name, **kwargs): self.field_name = field_name self.kwargs = kwargs @@ -99,6 +143,8 @@ class BaseField(object): class SearchField(BaseField): + searchable = True + def __init__(self, field_name, boost=None, partial_match=False, **kwargs): super(SearchField, self).__init__(field_name, **kwargs) self.boost = boost From 8bb3703c6eea10e4e02abab134d03131ec68e1d8 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:09:31 +0100 Subject: [PATCH 07/72] Created get_field_mapping method on ElasticSearchMapping class --- wagtail/wagtailsearch/backends/elasticsearch.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 28127fe83..ac8ee068b 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -19,6 +19,9 @@ class ElasticSearchMapping(object): def get_document_type(self): return self.model.indexed_get_content_type() + def get_field_mapping(self, field): + return field.get_attname(self.model), field.to_dict(self.model) + def get_mapping(self): # Make field list fields = { @@ -26,8 +29,9 @@ class ElasticSearchMapping(object): 'content_type': dict(type='string'), } - for field in self.model.get_search_fields(): - fields[field.get_attname(self.model)] = field.to_dict(self.model) + fields.update(dict( + self.get_field_mapping(field) for field in self.model.get_search_fields() + )) return { self.get_document_type(): { From 0aca9ece6576640cfd8bac0b508f3a2ffde3ce8b Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:29:09 +0100 Subject: [PATCH 08/72] Moved field mapping generation code into ElasticSearchMapping class --- .../wagtailsearch/backends/elasticsearch.py | 19 +++++++++-- wagtail/wagtailsearch/indexed.py | 32 +++---------------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index ac8ee068b..88542f624 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -8,7 +8,7 @@ from elasticsearch import Elasticsearch, NotFoundError, RequestError from elasticsearch.helpers import bulk from wagtail.wagtailsearch.backends.base import BaseSearch -from wagtail.wagtailsearch.indexed import Indexed +from wagtail.wagtailsearch.indexed import Indexed, SearchField, FilterField from wagtail.wagtailsearch.utils import normalise_query_string @@ -20,7 +20,22 @@ class ElasticSearchMapping(object): return self.model.indexed_get_content_type() def get_field_mapping(self, field): - return field.get_attname(self.model), field.to_dict(self.model) + mapping = {'type': 'string'} + + if isinstance(field, SearchField): + if field.boost: + mapping['boost'] = field.boost + + if field.partial_match: + mapping['analyzer'] = 'edgengram_analyzer' + elif isinstance(field, FilterField): + mapping['index'] = 'not_analyzed' + + if 'es_extra' in field.kwargs: + for key, value in field.kwargs['es_extra'].items(): + mapping[key] = value + + return field.get_index_name(self.model), mapping def get_mapping(self): # Make field list diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index b4e67f4c1..347e41c0c 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -112,6 +112,7 @@ class Indexed(object): class BaseField(object): searchable = False + suffix = '' def __init__(self, field_name, **kwargs): self.field_name = field_name @@ -127,16 +128,8 @@ class BaseField(object): except models.fields.FieldDoesNotExist: return self.field_name - def to_dict(self, cls): - dic = { - 'type': 'string' - } - - if 'es_extra' in self.kwargs: - for key, value in self.kwargs['es_extra'].items(): - dic[key] = value - - return dic + def get_index_name(self, cls): + return self.get_attname(cls) + self.suffix def __repr__(self): return "<%s: %s>" % (self.__class__.__name__, self.field_name) @@ -150,23 +143,6 @@ class SearchField(BaseField): self.boost = boost self.partial_match = partial_match - def to_dict(self, cls): - dic = super(SearchField, self).to_dict(cls) - - if self.boost and 'boost' not in dic: - dic['boost'] = self.boost - - if self.partial_match and 'analyzer' not in dic: - dic['analyzer'] = 'edgengram_analyzer' - - return dic - class FilterField(BaseField): - def to_dict(self, cls): - dic = super(FilterField, self).to_dict(cls) - - if 'index' not in dic: - dic['index'] = 'not_analyzed' - - return dic + suffix = '_filter' From 5e76a54b2b4046b4a6a177c04c3ad8fc3bc345fe Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:36:10 +0100 Subject: [PATCH 09/72] Remove any duplicate search fields of the same type --- wagtail/wagtailsearch/indexed.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index 347e41c0c..aba3003f7 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -101,6 +101,13 @@ class Indexed(object): # Add the field search_fields.append(SearchField(field_name, boost=boost, partial_match=partial_match, es_extra=config)) + # Remove any duplicate entries into search fields + # We need to take into account that fields can be indexed as both a SearchField and as a FilterField + search_fields_dict = {} + for field in search_fields: + search_fields_dict[(field.field_name, type(field))] = field + search_fields = search_fields_dict.values() + return search_fields @classmethod From 6d21727b034a1f0147c1be987aab27e4728a5199 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:48:51 +0100 Subject: [PATCH 10/72] Cleaned up database backend search method --- wagtail/wagtailsearch/backends/db.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailsearch/backends/db.py b/wagtail/wagtailsearch/backends/db.py index a7dca12e9..5ed8e07df 100644 --- a/wagtail/wagtailsearch/backends/db.py +++ b/wagtail/wagtailsearch/backends/db.py @@ -49,7 +49,7 @@ class DBSearch(BaseSearch): # Filter by terms for term in terms: - term_query = None + term_query = models.Q() for field_name in fields: # Check if the field exists (this will filter out indexed callables) try: @@ -58,11 +58,8 @@ class DBSearch(BaseSearch): continue # Filter on this field - field_filter = {'%s__icontains' % field_name: term} - if term_query is None: - term_query = models.Q(**field_filter) - else: - term_query |= models.Q(**field_filter) + term_query |= models.Q(**{'%s__icontains' % field_name: term}) + query = query.filter(term_query) # Distinct From 67ed563dd74602a1cfb79b2d1e274c3c8aa8cf2d Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:52:06 +0100 Subject: [PATCH 11/72] Made quotes more consistant --- .../wagtailsearch/backends/elasticsearch.py | 62 +++++++++---------- wagtail/wagtailsearch/indexed.py | 4 +- .../management/commands/update_index.py | 6 +- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 88542f624..4890b5e52 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -65,7 +65,7 @@ class ElasticSearchMapping(object): doc[field] = getattr(obj, field) # Check if this field is callable - if hasattr(doc[field], "__call__"): + if hasattr(doc[field], '__call__'): # Call it doc[field] = doc[field]() @@ -346,43 +346,43 @@ class ElasticSearch(BaseSearch): # Settings INDEX_SETTINGS = { - "settings": { - "analysis": { - "analyzer": { - "ngram_analyzer": { - "type": "custom", - "tokenizer": "lowercase", - "filter": ["ngram"] + 'settings': { + 'analysis': { + 'analyzer': { + 'ngram_analyzer': { + 'type': 'custom', + 'tokenizer': 'lowercase', + 'filter': ['ngram'] }, - "edgengram_analyzer": { - "type": "custom", - "tokenizer": "lowercase", - "filter": ["edgengram"] + 'edgengram_analyzer': { + 'type': 'custom', + 'tokenizer': 'lowercase', + 'filter': ['edgengram'] } }, - "tokenizer": { - "ngram_tokenizer": { - "type": "nGram", - "min_gram": 3, - "max_gram": 15, + 'tokenizer': { + 'ngram_tokenizer': { + 'type': 'nGram', + 'min_gram': 3, + 'max_gram': 15, }, - "edgengram_tokenizer": { - "type": "edgeNGram", - "min_gram": 2, - "max_gram": 15, - "side": "front" + 'edgengram_tokenizer': { + 'type': 'edgeNGram', + 'min_gram': 2, + 'max_gram': 15, + 'side': 'front' } }, - "filter": { - "ngram": { - "type": "nGram", - "min_gram": 3, - "max_gram": 15 + 'filter': { + 'ngram': { + 'type': 'nGram', + 'min_gram': 3, + 'max_gram': 15 }, - "edgengram": { - "type": "edgeNGram", - "min_gram": 1, - "max_gram": 15 + 'edgengram': { + 'type': 'edgeNGram', + 'min_gram': 1, + 'max_gram': 15 } } } diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index aba3003f7..b4626914f 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -49,7 +49,7 @@ class Indexed(object): if isinstance(indexed_fields, string_types): indexed_fields = [indexed_fields] if isinstance(indexed_fields, list): - indexed_fields = dict((field, dict(type="string")) for field in indexed_fields) + indexed_fields = dict((field, dict(type='string')) for field in indexed_fields) if not isinstance(indexed_fields, dict): raise ValueError() @@ -139,7 +139,7 @@ class BaseField(object): return self.get_attname(cls) + self.suffix def __repr__(self): - return "<%s: %s>" % (self.__class__.__name__, self.field_name) + return '<%s: %s>' % (self.__class__.__name__, self.field_name) class SearchField(BaseField): diff --git a/wagtail/wagtailsearch/management/commands/update_index.py b/wagtail/wagtailsearch/management/commands/update_index.py index 8f1605b24..ee85da8ff 100644 --- a/wagtail/wagtailsearch/management/commands/update_index.py +++ b/wagtail/wagtailsearch/management/commands/update_index.py @@ -25,13 +25,13 @@ class Command(BaseCommand): # Loop through objects for obj in model.objects.all(): - # Check if this object has an "object_indexed" function - if hasattr(obj, "object_indexed"): + # Check if this object has an 'object_indexed' function + if hasattr(obj, 'object_indexed'): if obj.object_indexed() is False: continue # Get key for this object - key = toplevel_content_type + ":" + str(obj.pk) + key = toplevel_content_type + ':' + str(obj.pk) # Check if this key already exists if key in object_set: From f2d1c803783a4e69e84d46b5f87dd58b41cfc770 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 6 Apr 2014 14:21:40 +0100 Subject: [PATCH 12/72] Don't run tests with wagtailsearch signal handlers enabled --- wagtail/tests/urls.py | 4 ---- wagtail/wagtailsearch/tests/test_backends.py | 10 +++++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/wagtail/tests/urls.py b/wagtail/tests/urls.py index 83e12adfb..385d0e6cf 100644 --- a/wagtail/tests/urls.py +++ b/wagtail/tests/urls.py @@ -6,10 +6,6 @@ from wagtail.wagtaildocs import urls as wagtaildocs_urls from wagtail.wagtailsearch.urls import frontend as wagtailsearch_frontend_urls from wagtail.contrib.wagtailsitemaps.views import sitemap -# Signal handlers -from wagtail.wagtailsearch import register_signal_handlers as wagtailsearch_register_signal_handlers -wagtailsearch_register_signal_handlers() - urlpatterns = patterns('', url(r'^admin/', include(wagtailadmin_urls)), diff --git a/wagtail/wagtailsearch/tests/test_backends.py b/wagtail/wagtailsearch/tests/test_backends.py index 8fab1d152..86a67f21a 100644 --- a/wagtail/wagtailsearch/tests/test_backends.py +++ b/wagtail/wagtailsearch/tests/test_backends.py @@ -11,11 +11,6 @@ from wagtail.wagtailsearch.backends.db import DBSearch from wagtail.wagtailsearch.backends import InvalidSearchBackendError -# Register wagtailsearch signal handlers -from wagtail.wagtailsearch import register_signal_handlers -register_signal_handlers() - - class BackendTests(object): # To test a specific backend, subclass BackendTests and define self.backend_path. @@ -41,21 +36,25 @@ class BackendTests(object): testa = models.SearchTest() testa.title = "Hello World" testa.save() + self.backend.add(testa) self.testa = testa testb = models.SearchTest() testb.title = "Hello" testb.live = True testb.save() + self.backend.add(testb) testc = models.SearchTestChild() testc.title = "Hello" testc.live = True testc.save() + self.backend.add(testc) testd = models.SearchTestChild() testd.title = "World" testd.save() + self.backend.add(testd) # Refresh the index self.backend.refresh_index() @@ -130,6 +129,7 @@ class BackendTests(object): def test_delete(self): # Delete one of the objects + self.backend.delete(self.testa) self.testa.delete() # Refresh index From 6847109bb994580e67ca33ba93f1c382ab81ec6e Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 29 Apr 2014 11:54:02 +0100 Subject: [PATCH 13/72] Removed 'object_indexed' check from update_index command Conflicts: wagtail/wagtailsearch/management/commands/update_index.py --- wagtail/wagtailsearch/management/commands/update_index.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/wagtail/wagtailsearch/management/commands/update_index.py b/wagtail/wagtailsearch/management/commands/update_index.py index ee85da8ff..0b4cb695f 100644 --- a/wagtail/wagtailsearch/management/commands/update_index.py +++ b/wagtail/wagtailsearch/management/commands/update_index.py @@ -25,11 +25,6 @@ class Command(BaseCommand): # Loop through objects for obj in model.objects.all(): - # Check if this object has an 'object_indexed' function - if hasattr(obj, 'object_indexed'): - if obj.object_indexed() is False: - continue - # Get key for this object key = toplevel_content_type + ':' + str(obj.pk) From b1fb9dc2e249024961323d6f34ce4ee970a698fe Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 15:59:39 +0100 Subject: [PATCH 14/72] Fixed a few more double quotes --- wagtail/wagtailsearch/indexed.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index b4626914f..4b636497b 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -15,13 +15,13 @@ class Indexed(object): @classmethod def indexed_get_content_type(cls): # Work out content type - content_type = (cls._meta.app_label + "_" + cls.__name__).lower() + content_type = (cls._meta.app_label + '_' + cls.__name__).lower() # Get parent content type parent = cls.indexed_get_parent() if parent: parent_content_type = parent.indexed_get_content_type() - return parent_content_type + "_" + content_type + return parent_content_type + '_' + content_type else: return content_type @@ -33,7 +33,7 @@ class Indexed(object): return parent.indexed_get_content_type() else: # At toplevel, return this content type - return (cls._meta.app_label + "_" + cls.__name__).lower() + return (cls._meta.app_label + '_' + cls.__name__).lower() @classmethod def indexed_get_indexed_fields(cls): From 4a70a4251b7ec5140351f7d2f255d3ccfc81894b Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 6 Apr 2014 14:41:40 +0100 Subject: [PATCH 15/72] Give some feedback from the add_bulk command --- wagtail/wagtailsearch/backends/db.py | 4 ++-- wagtail/wagtailsearch/backends/elasticsearch.py | 1 + wagtail/wagtailsearch/management/commands/update_index.py | 6 ++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailsearch/backends/db.py b/wagtail/wagtailsearch/backends/db.py index 5ed8e07df..4c05b19df 100644 --- a/wagtail/wagtailsearch/backends/db.py +++ b/wagtail/wagtailsearch/backends/db.py @@ -22,7 +22,7 @@ class DBSearch(BaseSearch): pass # Not needed def add_bulk(self, obj_list): - pass # Not needed + return [] # Not needed def delete(self, obj): pass # Not needed @@ -70,4 +70,4 @@ class DBSearch(BaseSearch): for prefetch in prefetch_related: query = query.prefetch_related(prefetch) - return query \ No newline at end of file + return query diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 4890b5e52..dbe755ff1 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -447,6 +447,7 @@ class ElasticSearch(BaseSearch): action.update(doc) actions.append(action) + yield type_name, len(type_documents) bulk(self.es, actions) def delete(self, obj): diff --git a/wagtail/wagtailsearch/management/commands/update_index.py b/wagtail/wagtailsearch/management/commands/update_index.py index 0b4cb695f..2c1593635 100644 --- a/wagtail/wagtailsearch/management/commands/update_index.py +++ b/wagtail/wagtailsearch/management/commands/update_index.py @@ -57,10 +57,8 @@ class Command(BaseCommand): # Add objects to index self.stdout.write("Adding objects") - results = s.add_bulk(object_set.values()) - if results: - for result in results: - self.stdout.write(result[0] + ' ' + str(result[1])) + for result in s.add_bulk(object_set.values()): + self.stdout.write(result[0] + ' ' + str(result[1])) # Refresh index self.stdout.write("Refreshing index") From 673da4ab021eb7f2fa14d9e194d12689373ba9c5 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 17:19:32 +0100 Subject: [PATCH 16/72] Set index:'not_analysed' setting on content_type field --- wagtail/wagtailsearch/backends/elasticsearch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index dbe755ff1..8fb706bba 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -41,7 +41,7 @@ class ElasticSearchMapping(object): # Make field list fields = { 'pk': dict(type='string', index='not_analyzed', store='yes'), - 'content_type': dict(type='string'), + 'content_type': dict(type='string', index='not_analyzed'), } fields.update(dict( From 584f5a7049a587f1da398ad421f9afd40a13fde8 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 16:22:35 +0100 Subject: [PATCH 17/72] Index fields with correct type in ElasticSearch Previously, everything was converted to a string before indexing in ElasticSearch. This caused issues where certian filters may not work as expected (such as a greater than filter on an integer field) This commit changes this by adding type conversion into the ElasticSearch backend. --- .../wagtailsearch/backends/elasticsearch.py | 38 ++++++++++++++----- wagtail/wagtailsearch/indexed.py | 20 ++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 8fb706bba..7ffebd045 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -13,6 +13,32 @@ from wagtail.wagtailsearch.utils import normalise_query_string class ElasticSearchMapping(object): + TYPE_MAP = { + 'AutoField': 'integer', + 'BinaryField': 'binary', + 'BooleanField': 'boolean', + 'CharField': 'string', + 'CommaSeparatedIntegerField': 'string', + 'DateField': 'date', + 'DateTimeField': 'date', + 'DecimalField': 'double', + 'FileField': 'string', + 'FilePathField': 'string', + 'FloatField': 'double', + 'IntegerField': 'integer', + 'BigIntegerField': 'long', + 'IPAddressField': 'string', + 'GenericIPAddressField': 'string', + 'NullBooleanField': 'boolean', + 'OneToOneField': 'integer', + 'PositiveIntegerField': 'integer', + 'PositiveSmallIntegerField': 'integer', + 'SlugField': 'string', + 'SmallIntegerField': 'integer', + 'TextField': 'string', + 'TimeField': 'date', + } + def __init__(self, model): self.model = model @@ -20,7 +46,7 @@ class ElasticSearchMapping(object): return self.model.indexed_get_content_type() def get_field_mapping(self, field): - mapping = {'type': 'string'} + mapping = {'type': self.TYPE_MAP.get(field.get_type(self.model), 'string')} if isinstance(field, SearchField): if field.boost: @@ -60,14 +86,8 @@ class ElasticSearchMapping(object): def get_document(self, obj): # Build document doc = dict(pk=str(obj.pk), content_type=self.model.indexed_get_content_type()) - for field in [field.get_attname(self.model) for field in self.model.get_search_fields()]: - if hasattr(obj, field): - doc[field] = getattr(obj, field) - - # Check if this field is callable - if hasattr(doc[field], '__call__'): - # Call it - doc[field] = doc[field]() + for field in self.model.get_search_fields(): + doc[field.get_index_name(self.model)] = field.get_value(obj) return doc diff --git a/wagtail/wagtailsearch/indexed.py b/wagtail/wagtailsearch/indexed.py index 4b636497b..b290f440b 100644 --- a/wagtail/wagtailsearch/indexed.py +++ b/wagtail/wagtailsearch/indexed.py @@ -138,6 +138,26 @@ class BaseField(object): def get_index_name(self, cls): return self.get_attname(cls) + self.suffix + def get_type(self, cls): + if 'type' in self.kwargs: + return self.kwargs['type'] + + try: + field = self.get_field(cls) + return field.get_internal_type() + except models.fields.FieldDoesNotExist: + return 'CharField' + + def get_value(self, obj): + try: + field = self.get_field(obj.__class__) + return field._get_val_from_obj(obj) + except models.fields.FieldDoesNotExist: + value = getattr(obj, self.field_name, None) + if hasattr(value, '__call__'): + value = value() + return value + def __repr__(self): return '<%s: %s>' % (self.__class__.__name__, self.field_name) From e1721030dcb8391617af28c3e4f04503862eb1b1 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 17:13:34 +0100 Subject: [PATCH 18/72] Exclude filter fields from _all. Explicitly include search fields in _all --- wagtail/wagtailsearch/backends/elasticsearch.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 7ffebd045..5e9e6e984 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -54,8 +54,11 @@ class ElasticSearchMapping(object): if field.partial_match: mapping['analyzer'] = 'edgengram_analyzer' + + mapping['include_in_all'] = True elif isinstance(field, FilterField): mapping['index'] = 'not_analyzed' + mapping['include_in_all'] = False if 'es_extra' in field.kwargs: for key, value in field.kwargs['es_extra'].items(): @@ -66,8 +69,8 @@ class ElasticSearchMapping(object): def get_mapping(self): # Make field list fields = { - 'pk': dict(type='string', index='not_analyzed', store='yes'), - 'content_type': dict(type='string', index='not_analyzed'), + 'pk': dict(type='string', index='not_analyzed', store='yes', include_in_all=False), + 'content_type': dict(type='string', index='not_analyzed', include_in_all=False), } fields.update(dict( From ca1fa1d93341f21f60d0fdde943c062f5c4b343c Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 20 Jun 2014 17:32:06 +0100 Subject: [PATCH 19/72] Index partials together into '_partials' --- wagtail/wagtailsearch/backends/elasticsearch.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 5e9e6e984..8fc2950dd 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -71,6 +71,7 @@ class ElasticSearchMapping(object): fields = { 'pk': dict(type='string', index='not_analyzed', store='yes', include_in_all=False), 'content_type': dict(type='string', index='not_analyzed', include_in_all=False), + '_partials': dict(type='string', analyzer='edgengram_analyzer', include_in_all=False), } fields.update(dict( @@ -89,8 +90,18 @@ class ElasticSearchMapping(object): def get_document(self, obj): # Build document doc = dict(pk=str(obj.pk), content_type=self.model.indexed_get_content_type()) + partials = [] for field in self.model.get_search_fields(): - doc[field.get_index_name(self.model)] = field.get_value(obj) + value = field.get_value(obj) + + doc[field.get_index_name(self.model)] = value + + # Check if this field should be added into _partials + if isinstance(field, SearchField) and field.partial_match: + partials.append(value) + + # Add partials to document + doc['_partials'] = partials return doc @@ -102,7 +113,7 @@ class ElasticSearchQuery(object): def __init__(self, model, query_string, fields=None, filters={}): self.model = model self.query_string = query_string - self.fields = fields or ['_all'] + self.fields = fields or ['_all', '_partials'] self.filters = filters def _get_filters(self): From 6e32b6cf9b296edc48730fba5ae3640f47b3bef5 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 14:18:19 +0100 Subject: [PATCH 20/72] Update Page.route API so that it returns a Page instance rather than an HttpResponse; catch the latter in the 'serve' view for backward compatibility --- wagtail/tests/fixtures/test.json | 25 +++++++++++++++++++- wagtail/tests/models.py | 13 ++++++++++ wagtail/wagtailcore/models.py | 2 +- wagtail/wagtailcore/tests/test_page_model.py | 21 ++++++++++++++-- wagtail/wagtailcore/views.py | 14 +++++++++-- 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/wagtail/tests/fixtures/test.json b/wagtail/tests/fixtures/test.json index 2d3db6e35..b769b7960 100644 --- a/wagtail/tests/fixtures/test.json +++ b/wagtail/tests/fixtures/test.json @@ -23,7 +23,7 @@ "model": "wagtailcore.page", "fields": { "title": "Welcome to the Wagtail test site!", - "numchild": 3, + "numchild": 4, "show_in_menus": false, "live": true, "depth": 2, @@ -257,6 +257,29 @@ } }, +{ + "pk": 10, + "model": "wagtailcore.page", + "fields": { + "title": "Old style route method", + "numchild": 0, + "show_in_menus": true, + "live": true, + "depth": 3, + "content_type": ["tests", "pagewitholdstyleroutemethod"], + "path": "000100010004", + "url_path": "/home/old-style-route/", + "slug": "old-style-route" + } +}, +{ + "pk": 10, + "model": "tests.pagewitholdstyleroutemethod", + "fields": { + "content": "

Test with old style route method

" + } +}, + { "pk": 1, "model": "wagtailcore.site", diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index ff6dc2dcc..0c369dd34 100644 --- a/wagtail/tests/models.py +++ b/wagtail/tests/models.py @@ -106,6 +106,19 @@ class SimplePage(Page): content = models.TextField() +class PageWithOldStyleRouteMethod(Page): + """ + Prior to Wagtail 0.4, the route() method on Page returned an HttpResponse + rather than a Page instance. As subclasses of Page may override route, + we need to continue accepting this convention (albeit as a deprecated API). + """ + content = models.TextField() + template = 'tests/simple_page.html' + + def route(self, request, path_components): + return self.serve(request) + + # Event page class EventPageCarouselItem(Orderable, CarouselItem): diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index cf33bcbfb..d8edbfc72 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -415,7 +415,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): else: # request is for this very page if self.live: - return self.serve(request) + return self else: raise Http404 diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index da9fc6174..0543bf32c 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -2,7 +2,7 @@ from django.test import TestCase, Client from django.http import HttpRequest, Http404 from wagtail.wagtailcore.models import Page, Site -from wagtail.tests.models import EventPage, EventIndex, SimplePage +from wagtail.tests.models import EventPage, EventIndex, SimplePage, PageWithOldStyleRouteMethod class TestSiteRouting(TestCase): @@ -136,8 +136,13 @@ class TestRouting(TestCase): request = HttpRequest() request.path = '/events/christmas/' - response = homepage.route(request, ['events', 'christmas']) + found_page = homepage.route(request, ['events', 'christmas']) + self.assertEqual(found_page, christmas_page) + def test_request_serving(self): + christmas_page = EventPage.objects.get(url_path='/home/events/christmas/') + request = HttpRequest() + response = christmas_page.serve(request) self.assertEqual(response.status_code, 200) self.assertEqual(response.context_data['self'], christmas_page) used_template = response.resolve_template(response.template_name) @@ -226,6 +231,18 @@ class TestServeView(TestCase): self.assertContains(response, 'Christmas') + def test_old_style_routing(self): + """ + Test that route() methods that return an HttpResponse are correctly handled + """ + response = self.client.get('/old-style-route/') + expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context['self'], expected_page) + self.assertEqual(response.templates[0].name, 'tests/simple_page.html') + + class TestStaticSitePaths(TestCase): def setUp(self): self.root_page = Page.objects.get(id=1) diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 1b1f6cf43..7377cf761 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -1,4 +1,6 @@ -from django.http import Http404 +import warnings + +from django.http import HttpResponse, Http404 def serve(request, path): @@ -8,4 +10,12 @@ def serve(request, path): raise Http404 path_components = [component for component in path.split('/') if component] - return request.site.root_page.specific.route(request, path_components) + page = request.site.root_page.specific.route(request, path_components) + if isinstance(page, HttpResponse): + warnings.warn( + "Page.route should return a Page, not an HttpResponse", + DeprecationWarning + ) + return page + + return page.serve(request) From 9cc3614097b1ea645eabf63a45fe6e2338a641af Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 23:00:20 +0100 Subject: [PATCH 21/72] Catch the deprecation warning for old-style routes during unit tests --- wagtail/wagtailcore/tests/test_page_model.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 0543bf32c..34f8356f2 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -1,3 +1,5 @@ +import warnings + from django.test import TestCase, Client from django.http import HttpRequest, Http404 @@ -235,9 +237,15 @@ class TestServeView(TestCase): """ Test that route() methods that return an HttpResponse are correctly handled """ - response = self.client.get('/old-style-route/') - expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') + with warnings.catch_warnings(record=True) as w: + response = self.client.get('/old-style-route/') + # Check that a DeprecationWarning has been triggered + self.assertEqual(len(w), 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) + self.assertTrue("Page.route should return a Page" in str(w[-1].message)) + + expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') self.assertEqual(response.status_code, 200) self.assertEqual(response.context['self'], expected_page) self.assertEqual(response.templates[0].name, 'tests/simple_page.html') From df15ece5dcef3962b5c0b2ee914018491bf20e9a Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 23:30:00 +0100 Subject: [PATCH 22/72] Implement before_serve_page hook --- wagtail/tests/wagtail_hooks.py | 8 ++++++++ wagtail/wagtailcore/tests/test_page_model.py | 4 ++++ wagtail/wagtailcore/views.py | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/wagtail/tests/wagtail_hooks.py b/wagtail/tests/wagtail_hooks.py index 13e3e46d2..4ad63e881 100644 --- a/wagtail/tests/wagtail_hooks.py +++ b/wagtail/tests/wagtail_hooks.py @@ -1,3 +1,5 @@ +from django.http import HttpResponse + from wagtail.wagtailcore import hooks from wagtail.wagtailcore.whitelist import attribute_rule, check_url, allow_without_attributes @@ -17,3 +19,9 @@ def whitelister_element_rules(): 'a': attribute_rule({'href': check_url, 'target': True}), } hooks.register('construct_whitelister_element_rules', whitelister_element_rules) + + +def block_googlebot(page, request): + if request.META.get('HTTP_USER_AGENT') == 'GoogleBot': + return HttpResponse("

bad googlebot no cookie

") +hooks.register('before_serve_page', block_googlebot) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 34f8356f2..ab5b9c148 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -250,6 +250,10 @@ class TestServeView(TestCase): self.assertEqual(response.context['self'], expected_page) self.assertEqual(response.templates[0].name, 'tests/simple_page.html') + def test_before_serve_hook(self): + response = self.client.get('/events/', HTTP_USER_AGENT='GoogleBot') + self.assertContains(response, 'bad googlebot no cookie') + class TestStaticSitePaths(TestCase): def setUp(self): diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 7377cf761..0872dd68f 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -2,6 +2,8 @@ import warnings from django.http import HttpResponse, Http404 +from wagtail.wagtailcore import hooks + def serve(request, path): # we need a valid Site object corresponding to this request (set in wagtail.wagtailcore.middleware.SiteMiddleware) @@ -18,4 +20,9 @@ def serve(request, path): ) return page + for fn in hooks.get_hooks('before_serve_page'): + result = fn(page, request) + if isinstance(result, HttpResponse): + return result + return page.serve(request) From 4af5f3d24d50ebaa0f65dba76b678115c3afca07 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 11:22:46 +0100 Subject: [PATCH 23/72] Add PageViewRestriction model, and apply permission check in the 'serve' view --- .../0005_create_page_view_restriction.py | 117 ++++++++++++++++++ wagtail/wagtailcore/models.py | 7 ++ wagtail/wagtailcore/wagtail_hooks.py | 11 ++ 3 files changed, 135 insertions(+) create mode 100644 wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py create mode 100644 wagtail/wagtailcore/wagtail_hooks.py diff --git a/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py b/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py new file mode 100644 index 000000000..f2d79573f --- /dev/null +++ b/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py @@ -0,0 +1,117 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'PageViewRestriction' + db.create_table('wagtailcore_pageviewrestriction', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('page', self.gf('django.db.models.fields.related.ForeignKey')(related_name='view_restrictions', to=orm['wagtailcore.Page'])), + ('password', self.gf('django.db.models.fields.CharField')(max_length=255)), + )) + db.send_create_signal('wagtailcore', ['PageViewRestriction']) + + + def backwards(self, orm): + # Deleting model 'PageViewRestriction' + db.delete_table('wagtailcore_pageviewrestriction') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Group']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Permission']"}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'wagtailcore.grouppagepermission': { + 'Meta': {'object_name': 'GroupPagePermission'}, + 'group': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'page_permissions'", 'to': "orm['auth.Group']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'group_permissions'", 'to': "orm['wagtailcore.Page']"}), + 'permission_type': ('django.db.models.fields.CharField', [], {'max_length': '20'}) + }, + 'wagtailcore.page': { + 'Meta': {'object_name': 'Page'}, + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'pages'", 'to': "orm['contenttypes.ContentType']"}), + 'depth': ('django.db.models.fields.PositiveIntegerField', [], {}), + 'expire_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'expired': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'go_live_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'has_unpublished_changes': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'live': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'numchild': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}), + 'owner': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'owned_pages'", 'null': 'True', 'to': "orm['auth.User']"}), + 'path': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}), + 'search_description': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'seo_title': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'show_in_menus': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'slug': ('django.db.models.fields.SlugField', [], {'max_length': '50'}), + 'title': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'url_path': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}) + }, + 'wagtailcore.pagerevision': { + 'Meta': {'object_name': 'PageRevision'}, + 'approved_go_live_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'content_json': ('django.db.models.fields.TextField', [], {}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'revisions'", 'to': "orm['wagtailcore.Page']"}), + 'submitted_for_moderation': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}) + }, + 'wagtailcore.pageviewrestriction': { + 'Meta': {'object_name': 'PageViewRestriction'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'view_restrictions'", 'to': "orm['wagtailcore.Page']"}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '255'}) + }, + 'wagtailcore.site': { + 'Meta': {'unique_together': "(('hostname', 'port'),)", 'object_name': 'Site'}, + 'hostname': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_default_site': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'port': ('django.db.models.fields.IntegerField', [], {'default': '80'}), + 'root_page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'sites_rooted_here'", 'to': "orm['wagtailcore.Page']"}) + } + } + + complete_apps = ['wagtailcore'] \ No newline at end of file diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index d8edbfc72..b035a0856 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -9,6 +9,8 @@ from modelcluster.models import ClusterableModel from django.db import models, connection, transaction from django.db.models import get_model, Q +from django.db.models.signals import post_save +from django.dispatch import receiver from django.http import Http404 from django.core.cache import cache from django.core.handlers.wsgi import WSGIRequest @@ -1150,3 +1152,8 @@ class PagePermissionTester(object): else: # no publishing required, so the already-tested 'add' permission is sufficient return True + + +class PageViewRestriction(models.Model): + page = models.ForeignKey('Page', related_name='view_restrictions') + password = models.CharField(max_length=255) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py new file mode 100644 index 000000000..1dd480001 --- /dev/null +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -0,0 +1,11 @@ +from django.http import HttpResponse + +from wagtail.wagtailcore import hooks +from wagtail.wagtailcore.models import PageViewRestriction + +def check_view_restrictions(page, request): + ancestors_and_self = list(page.get_ancestors().values_list('id', flat=True)) + [page] + restrictions = PageViewRestriction.objects.filter(page__in=ancestors_and_self) + for restriction in restrictions: + return HttpResponse("

Blocked due to view restriction

") +hooks.register('before_serve_page', check_view_restrictions) From 6f855d7fd86556956248ded90c7e27832b3a713d Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 15:34:27 +0100 Subject: [PATCH 24/72] simplified get_ancestors logic --- wagtail/wagtailcore/wagtail_hooks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 1dd480001..73c98a01d 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -4,8 +4,7 @@ from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction def check_view_restrictions(page, request): - ancestors_and_self = list(page.get_ancestors().values_list('id', flat=True)) + [page] - restrictions = PageViewRestriction.objects.filter(page__in=ancestors_and_self) + restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) for restriction in restrictions: return HttpResponse("

Blocked due to view restriction

") hooks.register('before_serve_page', check_view_restrictions) From d0e93f997f5caf8be28989d1907413f12c3e9b56 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 17:24:04 +0100 Subject: [PATCH 25/72] Delegate serving of the 'password required' response to the Page model --- wagtail/wagtailcore/models.py | 8 ++++++++ .../templates/wagtailcore/password_required.html | 9 +++++++++ wagtail/wagtailcore/wagtail_hooks.py | 4 +--- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 wagtail/wagtailcore/templates/wagtailcore/password_required.html diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index b035a0856..59c9eb861 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -811,6 +811,14 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): def get_prev_siblings(self, inclusive=False): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') + password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') + def serve_password_required_response(self, request, form): + return TemplateResponse(request, self.password_required_template, { + 'self': self, + 'request': request, + 'form': form, + }) + def get_navigation_menu_items(): # Get all pages that appear in the navigation menu: ones which have children, diff --git a/wagtail/wagtailcore/templates/wagtailcore/password_required.html b/wagtail/wagtailcore/templates/wagtailcore/password_required.html new file mode 100644 index 000000000..d4ee0b1fc --- /dev/null +++ b/wagtail/wagtailcore/templates/wagtailcore/password_required.html @@ -0,0 +1,9 @@ + + + + Password required + + +

Password required

+ + diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 73c98a01d..347194c32 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,10 +1,8 @@ -from django.http import HttpResponse - from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) for restriction in restrictions: - return HttpResponse("

Blocked due to view restriction

") + return page.serve_password_required_response(request, None) hooks.register('before_serve_page', check_view_restrictions) From e4008a814824a24f36c288f7309c4867b8919870 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 4 Jun 2014 15:28:03 +0100 Subject: [PATCH 26/72] implement form submission logic for the password form --- wagtail/wagtailcore/forms.py | 16 +++++++++++++ wagtail/wagtailcore/models.py | 18 +++++++++----- .../wagtailcore/password_required.html | 6 +++++ wagtail/wagtailcore/urls.py | 5 +++- wagtail/wagtailcore/views.py | 24 +++++++++++++++++++ wagtail/wagtailcore/wagtail_hooks.py | 10 +++++++- 6 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 wagtail/wagtailcore/forms.py diff --git a/wagtail/wagtailcore/forms.py b/wagtail/wagtailcore/forms.py new file mode 100644 index 000000000..1b83c8c94 --- /dev/null +++ b/wagtail/wagtailcore/forms.py @@ -0,0 +1,16 @@ +from django import forms + +class PasswordPageViewRestrictionForm(forms.Form): + password = forms.CharField(label="Password", widget=forms.PasswordInput) + return_url = forms.CharField(widget=forms.HiddenInput) + + def __init__(self, *args, **kwargs): + self.restriction = kwargs.pop('instance') + super(PasswordPageViewRestrictionForm, self).__init__(*args, **kwargs) + + def clean_password(self): + data = self.cleaned_data['password'] + if data != self.restriction.password: + raise forms.ValidationError("The password you have entered is not correct. Please try again.") + + return data diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 59c9eb861..08e1b612b 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -812,12 +812,18 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') - def serve_password_required_response(self, request, form): - return TemplateResponse(request, self.password_required_template, { - 'self': self, - 'request': request, - 'form': form, - }) + def serve_password_required_response(self, request, form, action_url): + """ + Serve a response indicating that the user has been denied access to view this page, + and must supply a password. + form = a Django form object containing the password input + (and zero or more hidden fields that also need to be output on the template) + action_url = URL that this form should be POSTed to + """ + context = self.get_context(request) + context['form'] = form + context['action_url'] = action_url + return TemplateResponse(request, self.password_required_template, context) def get_navigation_menu_items(): diff --git a/wagtail/wagtailcore/templates/wagtailcore/password_required.html b/wagtail/wagtailcore/templates/wagtailcore/password_required.html index d4ee0b1fc..d7601ca69 100644 --- a/wagtail/wagtailcore/templates/wagtailcore/password_required.html +++ b/wagtail/wagtailcore/templates/wagtailcore/password_required.html @@ -5,5 +5,11 @@

Password required

+

You need a password to access this page.

+
+ {% csrf_token %} + {{ form.as_p }} + +
diff --git a/wagtail/wagtailcore/urls.py b/wagtail/wagtailcore/urls.py index f0336fec4..9777dea8a 100644 --- a/wagtail/wagtailcore/urls.py +++ b/wagtail/wagtailcore/urls.py @@ -2,7 +2,10 @@ from django.conf.urls import url from wagtail.wagtailcore import views urlpatterns = [ - # All front-end views are handled through Wagtail's core.views.serve mechanism. + url(r'^_util/authenticate_with_password/(\d+)/(\d+)/$', views.authenticate_with_password, + name='wagtailcore_authenticate_with_password'), + + # Front-end page views are handled through Wagtail's core.views.serve mechanism. # Here we match a (possibly empty) list of path segments, each followed by # a '/'. If a trailing slash is not present, we leave CommonMiddleware to # handle it as usual (i.e. redirect it to the trailing slash version if diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 0872dd68f..bd12d7f97 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -1,8 +1,12 @@ import warnings from django.http import HttpResponse, Http404 +from django.shortcuts import get_object_or_404, redirect +from django.core.urlresolvers import reverse from wagtail.wagtailcore import hooks +from wagtail.wagtailcore.models import Page, PageViewRestriction +from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def serve(request, path): @@ -26,3 +30,23 @@ def serve(request, path): return result return page.serve(request) + + +def authenticate_with_password(request, page_view_restriction_id, page_id): + """ + Handle a submission of PasswordPageViewRestrictionForm to grant view access over a + subtree that is protected by a PageViewRestriction + """ + restriction = get_object_or_404(PageViewRestriction, id=page_view_restriction_id) + page = get_object_or_404(Page, id=page_id).specific + + if request.POST: + form = PasswordPageViewRestrictionForm(request.POST, instance=restriction) + if form.is_valid(): + # TODO: record 'has authenticated against this page view restriction' flag in the session + return redirect(form.cleaned_data['return_url']) + else: + form = PasswordPageViewRestrictionForm(instance=restriction) + + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 347194c32..ccac0bd43 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,8 +1,16 @@ +from django.core.urlresolvers import reverse + from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction +from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) + for restriction in restrictions: - return page.serve_password_required_response(request, None) + form = PasswordPageViewRestrictionForm(instance=restriction, + initial={'return_url': request.get_full_path()}) + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) + hooks.register('before_serve_page', check_view_restrictions) From 1e71e10cd22d18e170d45d05349b8427358c7153 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 4 Jun 2014 16:02:43 +0100 Subject: [PATCH 27/72] Keep track of passed page view restrictions in the session --- wagtail/wagtailcore/views.py | 11 ++++++++++- wagtail/wagtailcore/wagtail_hooks.py | 13 ++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index bd12d7f97..78e0476b6 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -3,6 +3,7 @@ import warnings from django.http import HttpResponse, Http404 from django.shortcuts import get_object_or_404, redirect from django.core.urlresolvers import reverse +from django.conf import settings from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import Page, PageViewRestriction @@ -43,7 +44,15 @@ def authenticate_with_password(request, page_view_restriction_id, page_id): if request.POST: form = PasswordPageViewRestrictionForm(request.POST, instance=restriction) if form.is_valid(): - # TODO: record 'has authenticated against this page view restriction' flag in the session + has_existing_session = (settings.SESSION_COOKIE_NAME in request.COOKIES) + passed_restrictions = request.session.setdefault('passed_page_view_restrictions', []) + if restriction.id not in passed_restrictions: + passed_restrictions.append(restriction.id) + if not has_existing_session: + # if this is a session we've created, set it to expire at the end + # of the browser session + request.session.set_expiry(0) + return redirect(form.cleaned_data['return_url']) else: form = PasswordPageViewRestrictionForm(instance=restriction) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index ccac0bd43..b2ab31248 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -7,10 +7,13 @@ from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) - for restriction in restrictions: - form = PasswordPageViewRestrictionForm(instance=restriction, - initial={'return_url': request.get_full_path()}) - action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) - return page.serve_password_required_response(request, form, action_url) + if restrictions: + passed_restrictions = request.session.get('passed_page_view_restrictions', []) + for restriction in restrictions: + if restriction.id not in passed_restrictions: + form = PasswordPageViewRestrictionForm(instance=restriction, + initial={'return_url': request.get_full_path()}) + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) hooks.register('before_serve_page', check_view_restrictions) From cae0d8fecbcfb428ecc0130045ab6bdfee463269 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 5 Jun 2014 11:04:37 +0100 Subject: [PATCH 28/72] add public/private indicator on explorer and page editor --- .../wagtailadmin/pages/_view_permission_indicator.html | 6 ++++++ wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html | 3 +++ wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html | 5 ++++- wagtail/wagtailcore/models.py | 3 +++ wagtail/wagtailcore/wagtail_hooks.py | 3 +-- 5 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html new file mode 100644 index 000000000..7f9c7d82b --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html @@ -0,0 +1,6 @@ +{% load i18n %} +{% if page.get_view_restrictions %} +
{% trans 'Private' %}
+{% else %} +
{% trans 'Public' %}
+{% endif %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html index 8cc7fdca6..e56ad170c 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html @@ -16,6 +16,9 @@
{% trans "Status:" %} {% if page.live %}{{ page.status_string }}{% else %}{{ page.status_string }}{% endif %}
+ + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page only %} + diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html index 7258c5429..5fe32838e 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html @@ -48,6 +48,9 @@ {% else %}

{{ parent_page.title }}

{% endif %} + + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=parent_page only %} +
    {% if parent_page_perms.can_edit and 'edit' not in hide_actions|default:'' %}
  • {% trans 'Edit' %}
  • @@ -249,4 +252,4 @@
-{% endif %} \ No newline at end of file +{% endif %} diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 08e1b612b..8670f0315 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -811,6 +811,9 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): def get_prev_siblings(self, inclusive=False): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') + def get_view_restrictions(self): + return PageViewRestriction.objects.filter(page__in=self.get_ancestors(inclusive=True)) + password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') def serve_password_required_response(self, request, form, action_url): """ diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index b2ab31248..e92971c99 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,11 +1,10 @@ from django.core.urlresolvers import reverse from wagtail.wagtailcore import hooks -from wagtail.wagtailcore.models import PageViewRestriction from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): - restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) + restrictions = page.get_view_restrictions() if restrictions: passed_restrictions = request.session.get('passed_page_view_restrictions', []) From eaee2b9695842f4c38a79874a55eec1e0aeb4065 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 6 Jun 2014 12:52:34 +0100 Subject: [PATCH 29/72] Implement (mostly-empty) views for setting view restrictions, accessible from the permission indicator --- .../js/view-permission-indicator.js | 9 +++++ .../ancestor_restriction.html | 8 +++++ .../set_view_restrictions.html | 7 ++++ .../wagtailadmin/pages/_editor_js.html | 1 + .../pages/_view_permission_indicator.html | 25 ++++++++++--- .../templates/wagtailadmin/pages/edit.html | 6 ++-- .../templates/wagtailadmin/pages/index.html | 4 +++ .../templates/wagtailadmin/pages/list.html | 2 +- wagtail/wagtailadmin/urls.py | 4 ++- .../views/page_view_restrictions.py | 35 +++++++++++++++++++ wagtail/wagtailcore/models.py | 3 ++ 11 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html create mode 100644 wagtail/wagtailadmin/views/page_view_restrictions.py diff --git a/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js b/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js new file mode 100644 index 000000000..52b7442a7 --- /dev/null +++ b/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js @@ -0,0 +1,9 @@ +$(function() { + /* Interface to set view permissions from the explorer / editor */ + $('a.action-set-view-permissions').click(function() { + ModalWorkflow({ + 'url': this.href + }); + return false; + }); +}); diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html new file mode 100644 index 000000000..d8b652f32 --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html @@ -0,0 +1,8 @@ +{% load i18n %} +{% trans "Access restricted" as title_str %} +{% include "wagtailadmin/shared/header.html" with title=title_str %} + +
+ {% trans "Access to this page is restricted because it is within the section:" %} + {{ page_with_restriction.title }} +
diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html new file mode 100644 index 000000000..82e928c68 --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html @@ -0,0 +1,7 @@ +{% load i18n %} +{% trans "Set access restrictions" as title_str %} +{% include "wagtailadmin/shared/header.html" with title=title_str %} + +
+

form for setting access restrictions goes here

+
diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html index 17bcbd8bf..f494f75ec 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html @@ -21,6 +21,7 @@ + {% hook_output 'insert_editor_js' %} {% endcompress %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html index 7f9c7d82b..030703aab 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html @@ -1,6 +1,21 @@ -{% load i18n %} -{% if page.get_view_restrictions %} -
{% trans 'Private' %}
-{% else %} -
{% trans 'Public' %}
+{% load i18n wagtailadmin_tags %} + +{% if not page_perms %} + {% page_permissions page as page_perms %} {% endif %} + +{% with page.get_view_restrictions as has_view_restrictions %} + {% if has_view_restrictions %} + {% trans 'Private' as label %} + {% else %} + {% trans 'Public' as label %} + {% endif %} + + {% if page_perms.can_set_view_restrictions %} + + {% else %} +
{{ label }}
+ {% endif %} +{% endwith %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html index e56ad170c..a1b108933 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html @@ -6,6 +6,7 @@ {% block bodyclass %}menu-explorer page-editor{% endblock %} {% block content %} + {% page_permissions page as page_perms %}
{% include "wagtailadmin/shared/breadcrumb.html" with page=page %} @@ -17,7 +18,7 @@ {% trans "Status:" %} {% if page.live %}{{ page.status_string }}{% else %}{{ page.status_string }}{% endif %} - {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page only %} + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page page_perms=page_perms only %}
@@ -25,8 +26,7 @@
{% csrf_token %} {{ edit_handler.render_form_content }} - - {% page_permissions page as page_perms %} +