diff --git a/TODO.md b/TODO.md index 40ba2b24..2e366607 100644 --- a/TODO.md +++ b/TODO.md @@ -35,3 +35,9 @@ TODO * git deploy in addition to FTP? * env vars instead of multiple settings files: https://devcenter.heroku.com/articles/config-vars ? * optional chroot shell? + +* make sure prefetch_related() is used correctly +Remember that, as always with QuerySets, any subsequent chained methods which imply a different database query will ignore previously cached results, and retrieve data using a fresh database query. +* profile select_related vs prefetch_related + +* use HTTP OPTIONS instead of configuration endpoint, or rename to settings? diff --git a/orchestra/api/options.py b/orchestra/api/options.py index 30c71a21..c540784e 100644 --- a/orchestra/api/options.py +++ b/orchestra/api/options.py @@ -1,13 +1,12 @@ -from django import conf from django.core.exceptions import ImproperlyConfigured -from rest_framework import views -from rest_framework.response import Response -from rest_framework.reverse import reverse from rest_framework.routers import DefaultRouter, Route, flatten, replace_methodname -from .. import settings -from ..utils.apps import autodiscover as module_autodiscover +from orchestra import settings +from orchestra.utils.apps import autodiscover as module_autodiscover +from orchestra.utils.python import import_class + from .helpers import insert_links, replace_collectionmethodname +from .root import APIRoot def collectionlink(**kwargs): @@ -79,39 +78,8 @@ class LinkHeaderRouter(DefaultRouter): def get_api_root_view(self): """ returns the root view, with all the linked collections """ - class APIRoot(views.APIView): - def get(instance, request, format=None): - root_url = reverse('api-root', request=request, format=format) - token_url = reverse('api-token-auth', request=request, format=format) - links = [ - '<%s>; rel="%s"' % (root_url, 'api-root'), - '<%s>; rel="%s"' % (token_url, 'api-get-auth-token'), - ] - if not request.user.is_anonymous(): - list_name = '{basename}-list' - detail_name = '{basename}-detail' - for prefix, viewset, basename in self.registry: - singleton_pk = getattr(viewset, 'singleton_pk', False) - if singleton_pk: - url_name = detail_name.format(basename=basename) - kwargs = { 'pk': singleton_pk(viewset(), request) } - else: - url_name = list_name.format(basename=basename) - kwargs = {} - url = reverse(url_name, request=request, format=format, kwargs=kwargs) - links.append('<%s>; rel="%s"' % (url, url_name)) - # Add user link - url_name = detail_name.format(basename='user') - kwargs = { 'pk': request.user.pk } - url = reverse(url_name, request=request, format=format, kwargs=kwargs) - links.append('<%s>; rel="%s"' % (url, url_name)) - headers = { 'Link': ', '.join(links) } - content = { - name: getattr(settings, name, None) - for name in ['SITE_NAME', 'SITE_VERBOSE_NAME'] - } - content['INSTALLED_APPS'] = conf.settings.INSTALLED_APPS - return Response(content, headers=headers) + APIRoot = import_class(settings.API_ROOT_VIEW) + APIRoot.router = self return APIRoot.as_view() def register(self, prefix, viewset, base_name=None): diff --git a/orchestra/apps/domains/api.py b/orchestra/apps/domains/api.py index 989a77bc..b5089bec 100644 --- a/orchestra/apps/domains/api.py +++ b/orchestra/apps/domains/api.py @@ -2,7 +2,7 @@ from rest_framework import viewsets from rest_framework.decorators import link from rest_framework.response import Response -from orchestra.api import router, collectionlink +from orchestra.api import router from orchestra.apps.accounts.api import AccountApiMixin from . import settings @@ -19,17 +19,18 @@ class DomainViewSet(AccountApiMixin, viewsets.ModelViewSet): qs = super(DomainViewSet, self).get_queryset() return qs.prefetch_related('records') - @collectionlink() - def configuration(self, request): - names = ['DOMAINS_DEFAULT_A', 'DOMAINS_DEFAULT_MX', 'DOMAINS_DEFAULT_NS'] - return Response({ - name: getattr(settings, name, None) for name in names - }) - @link() def view_zone(self, request, pk=None): domain = self.get_object() return Response({'zone': domain.render_zone()}) + + def metadata(self, request): + ret = super(DomainViewSet, self).metadata(request) + names = ['DOMAINS_DEFAULT_A', 'DOMAINS_DEFAULT_MX', 'DOMAINS_DEFAULT_NS'] + ret['settings'] = { + name.lower(): getattr(settings, name, None) for name in names + } + return ret router.register(r'domains', DomainViewSet) diff --git a/orchestra/apps/domains/backends.py b/orchestra/apps/domains/backends.py index aea7206b..8e84c097 100644 --- a/orchestra/apps/domains/backends.py +++ b/orchestra/apps/domains/backends.py @@ -75,6 +75,7 @@ class Bind9MasterDomainBackend(ServiceBackend): class Bind9SlaveDomainBackend(Bind9MasterDomainBackend): verbose_name = _("Bind9 slave domain") related_models = (('domains.Domain', 'origin'),) + def save(self, domain): context = self.get_context(domain) self.update_conf(context) diff --git a/orchestra/apps/issues/admin.py b/orchestra/apps/issues/admin.py index 6c7cc56e..7b867e85 100644 --- a/orchestra/apps/issues/admin.py +++ b/orchestra/apps/issues/admin.py @@ -8,12 +8,12 @@ from django.db import models from django.http import HttpResponse from django.shortcuts import get_object_or_404 from django.utils.html import strip_tags -from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ from markdown import markdown from orchestra.admin import ChangeListDefaultFilter, ExtendedModelAdmin#, ChangeViewActions from orchestra.admin.utils import (link, colored, wrap_admin_view, display_timesince) +from orchestra.apps.contacts import settings as contacts_settings from .actions import (reject_tickets, resolve_tickets, take_tickets, close_tickets, mark_as_unread, mark_as_read, set_default_queue) @@ -52,15 +52,15 @@ class MessageReadOnlyInline(admin.TabularInline): 'all': ('orchestra/css/hide-inline-id.css',) } - def content_html(self, obj): + def content_html(self, msg): context = { - 'number': obj.number, - 'time': display_timesince(obj.created_on), - 'author': link('author')(self, obj), + 'number': msg.number, + 'time': display_timesince(msg.created_on), + 'author': link('author')(self, msg) if msg.author else msg.author_name, } summary = _("#%(number)i Updated by %(author)s about %(time)s") % context header = '%s
' % summary - content = markdown(obj.content) + content = markdown(msg.content) content = content.replace('>\n', '>') content = '
%s
' % content return header + content @@ -111,8 +111,9 @@ class TicketInline(admin.TabularInline): owner_link = link('owner') def ticket_id(self, instance): - return mark_safe('%s' % link()(self, instance)) + return '%s' % link()(self, instance) ticket_id.short_description = '#' + ticket_id.allow_tags = True def colored_state(self, instance): return colored('state', STATE_COLORS, bold=False)(instance) @@ -165,7 +166,6 @@ class TicketAdmin(ChangeListDefaultFilter, ExtendedModelAdmin): #TODO ChangeView ) readonly_fieldsets = ( (None, { - 'classes': ('wide',), 'fields': ('display_summary', ('display_queue', 'display_owner'), ('display_state', 'display_priority'), @@ -174,7 +174,7 @@ class TicketAdmin(ChangeListDefaultFilter, ExtendedModelAdmin): #TODO ChangeView ) fieldsets = readonly_fieldsets + ( ('Update', { - 'classes': ('collapse', 'wide'), + 'classes': ('collapse',), 'fields': ('subject', ('queue', 'owner',), ('state', 'priority'), @@ -183,7 +183,6 @@ class TicketAdmin(ChangeListDefaultFilter, ExtendedModelAdmin): #TODO ChangeView ) add_fieldsets = ( (None, { - 'classes': ('wide',), 'fields': ('subject', ('queue', 'owner',), ('state', 'priority'), @@ -204,17 +203,21 @@ class TicketAdmin(ChangeListDefaultFilter, ExtendedModelAdmin): #TODO ChangeView display_owner = link('owner') def display_summary(self, ticket): - author_url = link('creator')(self, ticket) - created = display_timesince(ticket.created_on) - messages = ticket.messages.order_by('-created_on') - updated = '' - if messages: - updated_on = display_timesince(messages[0].created_on) - updated_by = link('author')(self, messages[0]) - updated = '. Updated by %s about %s' % (updated_by, updated_on) - msg = '

Added by %s about %s%s

' % (author_url, created, updated) - return mark_safe(msg) + context = { + 'creator': link('creator')(self, ticket) if ticket.creator else ticket.creator_name, + 'created': display_timesince(ticket.created_on), + 'updated': '', + } + msg = ticket.messages.last() + if msg: + context.update({ + 'updated': display_timesince(msg.created_on), + 'updater': link('author')(self, msg) if msg.author else msg.author_name, + }) + context['updated'] = '. Updated by %(updater)s about %(updated)s' % context + return '

Added by %(creator)s about %(created)s%(updated)s

' % context display_summary.short_description = 'Summary' + display_summary.allow_tags = True def display_priority(self, ticket): """ State colored for change_form """ @@ -302,15 +305,17 @@ class TicketAdmin(ChangeListDefaultFilter, ExtendedModelAdmin): #TODO ChangeView def message_preview_view(self, request): """ markdown preview render via ajax """ data = request.POST.get("data") - data_formated = markdown(strip_tags(data)) + data_formated = markdowt_tn(strip_tags(data)) return HttpResponse(data_formated) + + def queryset(self, request): + """ Order by structured name and imporve performance """ + qs = super(TicketAdmin, self).queryset(request) + return qs.select_related('queue', 'owner', 'creator') class QueueAdmin(admin.ModelAdmin): - # TODO notify - list_display = [ - 'name', 'default', 'num_tickets' - ] + list_display = ['name', 'default', 'num_tickets'] actions = [set_default_queue] inlines = [TicketInline] ordering = ['name'] @@ -324,9 +329,21 @@ class QueueAdmin(admin.ModelAdmin): num = queue.tickets.count() url = reverse('admin:issues_ticket_changelist') url += '?my_tickets=False&queue=%i' % queue.pk - return mark_safe('%d' % (url, num)) + return '%d' % (url, num) num_tickets.short_description = _("Tickets") num_tickets.admin_order_field = 'tickets__count' + num_tickets.allow_tags = True + + def get_list_display(self, request): + """ show notifications """ + list_display = list(self.list_display) + for value, verbose in contacts_settings.CONTACTS_EMAIL_USAGES: + def display_notify(queue, notify=value): + return notify in queue.notify + display_notify.short_description = verbose + display_notify.boolean = True + list_display.append(display_notify) + return list_display def queryset(self, request): qs = super(QueueAdmin, self).queryset(request) diff --git a/orchestra/apps/issues/api.py b/orchestra/apps/issues/api.py index 94630552..aa811c3f 100644 --- a/orchestra/apps/issues/api.py +++ b/orchestra/apps/issues/api.py @@ -16,17 +16,19 @@ class TicketViewSet(viewsets.ModelViewSet): @action() def mark_as_read(self, request, pk=None): ticket = self.get_object() - ticket.mark_as_read() - return Response({'status': 'Ticket marked as readed'}) + ticket.mark_as_read_by(request.user) + return Response({'status': 'Ticket marked as read'}) @action() def mark_as_unread(self, request, pk=None): ticket = self.get_object() - ticket.mark_as_unread() - return Response({'status': 'Ticket marked as unreaded'}) + ticket.mark_as_unread_by(request.user) + return Response({'status': 'Ticket marked as unread'}) def get_queryset(self): qs = super(TicketViewSet, self).get_queryset() + qs = qs.select_related('creator', 'queue') + qs = qs.prefetch_related('messages__author') return qs.filter(creator__account=self.request.user.account_id) diff --git a/orchestra/apps/issues/forms.py b/orchestra/apps/issues/forms.py index 42984681..a051e7d7 100644 --- a/orchestra/apps/issues/forms.py +++ b/orchestra/apps/issues/forms.py @@ -92,7 +92,7 @@ class TicketForm(forms.ModelForm): description = description.replace('>\n', '#Ha9G9-?8') description = description.replace('\n', '
') description = description.replace('#Ha9G9-?8', '>\n') - description = '
%s
' % description + description = '
%s
' % description widget = ReadOnlyWidget(description, description) self.fields['display_description'].widget = widget diff --git a/orchestra/apps/issues/helpers.py b/orchestra/apps/issues/helpers.py index d15f4325..68df9c49 100644 --- a/orchestra/apps/issues/helpers.py +++ b/orchestra/apps/issues/helpers.py @@ -32,5 +32,9 @@ def get_ticket_changes(modeladmin, request, ticket): old_value = getattr(ticket, attr) new_value = form.cleaned_data[attr] if old_value != new_value: + choices = dict(form.fields[attr].choices) + if old_value in choices: + old_value = choices[old_value] + new_value = choices[new_value] changes[attr] = (old_value, new_value) return changes diff --git a/orchestra/apps/issues/models.py b/orchestra/apps/issues/models.py index 0c8eb749..48ccfd4a 100644 --- a/orchestra/apps/issues/models.py +++ b/orchestra/apps/issues/models.py @@ -57,7 +57,8 @@ class Ticket(models.Model): ) creator = models.ForeignKey(get_user_model(), verbose_name=_("created by"), - related_name='tickets_created') + related_name='tickets_created', null=True) + creator_name = models.CharField(_("creator name"), max_length=256, blank=True) owner = models.ForeignKey(get_user_model(), null=True, blank=True, related_name='tickets_owned', verbose_name=_("assigned to")) queue = models.ForeignKey(Queue, related_name='tickets', null=True, blank=True) @@ -104,6 +105,8 @@ class Ticket(models.Model): def save(self, *args, **kwargs): """ notify stakeholders of new ticket """ new_issue = not self.pk + if not self.creator_name and self.creator: + self.creator_name = self.creator.get_full_name() super(Ticket, self).save(*args, **kwargs) if new_issue: # PK should be available for rendering the template @@ -119,16 +122,16 @@ class Ticket(models.Model): return self.cc.split(',') if self.cc else [] def mark_as_read_by(self, user): - TicketTracker.objects.get_or_create(ticket=self, user=user) + self.trackers.get_or_create(user=user) def mark_as_unread_by(self, user): - TicketTracker.objects.filter(ticket=self, user=user).delete() + self.trackers.filter(user=user).delete() def mark_as_unread(self): - TicketTracker.objects.filter(ticket=self).delete() + self.trackers.all().delete() def is_read_by(self, user): - return TicketTracker.objects.filter(ticket=self, user=user).exists() + return self.trackers.filter(user=user).exists() def reject(self): self.state = Ticket.REJECTED @@ -152,9 +155,13 @@ class Message(models.Model): related_name='messages') author = models.ForeignKey(get_user_model(), verbose_name=_("author"), related_name='ticket_messages') + author_name = models.CharField(_("author name"), max_length=256, blank=True) content = models.TextField(_("content")) created_on = models.DateTimeField(_("created on"), auto_now_add=True) + class Meta: + get_latest_by = "created_on" + def __unicode__(self): return u"#%i" % self.id @@ -162,7 +169,9 @@ class Message(models.Model): """ notify stakeholders of ticket update """ if not self.pk: self.ticket.mark_as_unread() + self.ticket.mark_as_read_by(self.author) self.ticket.notify(message=self) + self.author_name = self.author.get_full_name() super(Message, self).save(*args, **kwargs) @property diff --git a/orchestra/apps/issues/serializers.py b/orchestra/apps/issues/serializers.py index 817bad05..cf411a12 100644 --- a/orchestra/apps/issues/serializers.py +++ b/orchestra/apps/issues/serializers.py @@ -13,8 +13,8 @@ class QueueSerializer(serializers.HyperlinkedModelSerializer): class MessageSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = Message - fields = ('id', 'author', 'content', 'created_on') - read_only_fields = ('author', 'created_on') + fields = ('id', 'author', 'author_name', 'content', 'created_on') + read_only_fields = ('author', 'author_name', 'created_on') def get_identity(self, data): return data.get('id') @@ -32,10 +32,10 @@ class TicketSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = Ticket fields = ( - 'url', 'id', 'creator', 'owner', 'queue', 'subject', 'description', - 'state', 'messages', 'is_read' + 'url', 'id', 'creator', 'creator_name', 'owner', 'queue', 'subject', + 'description', 'state', 'messages', 'is_read' ) - read_only_fields = ('creator', 'owner') + read_only_fields = ('creator', 'creator_name', 'owner') def get_is_read(self, obj): return obj.is_read_by(self.context['request'].user) diff --git a/orchestra/apps/orchestration/manager.py b/orchestra/apps/orchestration/manager.py index ff8aee30..fdd22e04 100644 --- a/orchestra/apps/orchestration/manager.py +++ b/orchestra/apps/orchestration/manager.py @@ -2,17 +2,12 @@ import threading from django import db +from orchestra.utils.python import import_class + from . import settings from .helpers import send_report -def get_router(): - module = '.'.join(settings.ORCHESTRATION_ROUTER.split('.')[:-1]) - cls = settings.ORCHESTRATION_ROUTER.split('.')[-1] - module = __import__(module, fromlist=[module]) - return getattr(module, cls) - - def as_task(execute): def wrapper(*args, **kwargs): with db.transaction.commit_manually(): @@ -37,7 +32,7 @@ def close_connection(execute): def execute(operations): """ generates and executes the operations on the servers """ - router = get_router() + router = import_class(settings.ORCHESTRATION_ROUTER) # Generate scripts per server+backend scripts = {} for operation in operations: diff --git a/orchestra/apps/users/models.py b/orchestra/apps/users/models.py index e1ba1d66..ef6f12be 100644 --- a/orchestra/apps/users/models.py +++ b/orchestra/apps/users/models.py @@ -37,9 +37,8 @@ class User(auth.AbstractBaseUser): REQUIRED_FIELDS = ['email'] def get_full_name(self): - """ Returns the first_name plus the last_name, with a space in between """ full_name = '%s %s' % (self.first_name, self.last_name) - return full_name.strip() + return full_name.strip() or self.username def get_short_name(self): """ Returns the short name for the user """ diff --git a/orchestra/apps/webapps/api.py b/orchestra/apps/webapps/api.py index 3e8eac7d..cf938e00 100644 --- a/orchestra/apps/webapps/api.py +++ b/orchestra/apps/webapps/api.py @@ -1,7 +1,7 @@ from rest_framework import viewsets from rest_framework.response import Response -from orchestra.api import router, collectionlink +from orchestra.api import router from orchestra.apps.accounts.api import AccountApiMixin from . import settings @@ -14,15 +14,16 @@ class WebAppViewSet(AccountApiMixin, viewsets.ModelViewSet): serializer_class = WebAppSerializer filter_fields = ('name',) - @collectionlink() - def configuration(self, request): + def metadata(self, request): + ret = super(WebAppViewSet, self).metadata(request) names = [ 'WEBAPPS_BASE_ROOT', 'WEBAPPS_TYPES', 'WEBAPPS_WEBAPP_OPTIONS', 'WEBAPPS_PHP_DISABLED_FUNCTIONS', 'WEBAPPS_DEFAULT_TYPE' ] - return Response({ - name: getattr(settings, name, None) for name in names - }) + ret['settings'] = { + name.lower(): getattr(settings, name, None) for name in names + } + return ret router.register(r'webapps', WebAppViewSet) diff --git a/orchestra/apps/websites/api.py b/orchestra/apps/websites/api.py index 7936cf2c..090076c7 100644 --- a/orchestra/apps/websites/api.py +++ b/orchestra/apps/websites/api.py @@ -1,7 +1,7 @@ from rest_framework import viewsets from rest_framework.response import Response -from orchestra.api import router, collectionlink +from orchestra.api import router from orchestra.apps.accounts.api import AccountApiMixin from . import settings @@ -14,12 +14,13 @@ class WebsiteViewSet(AccountApiMixin, viewsets.ModelViewSet): serializer_class = WebsiteSerializer filter_fields = ('name',) - @collectionlink() - def configuration(self, request): + def metadata(self, request): + ret = super(WebsiteViewSet, self).metadata(request) names = ['WEBSITES_OPTIONS', 'WEBSITES_PORT_CHOICES'] - return Response({ - name: getattr(settings, name, None) for name in names - }) + ret['settings'] = { + name.lower(): getattr(settings, name, None) for name in names + } + return ret router.register(r'websites', WebsiteViewSet) diff --git a/orchestra/settings.py b/orchestra/settings.py index 75b8585b..0663969d 100644 --- a/orchestra/settings.py +++ b/orchestra/settings.py @@ -26,3 +26,5 @@ STOP_SERVICES = getattr(settings, 'STOP_SERVICES', [('uwsgi', 'nginx'), 'celerybeat', 'celeryd', 'celeryevcam', 'postgresql'] ) + +API_ROOT_VIEW = getattr(settings, 'API_ROOT_VIEW', 'orchestra.api.root.APIRoot') diff --git a/orchestra/utils/python.py b/orchestra/utils/python.py index adba5e05..c90a0ba1 100644 --- a/orchestra/utils/python.py +++ b/orchestra/utils/python.py @@ -1,6 +1,13 @@ import collections +def import_class(cls): + module = '.'.join(cls.split('.')[:-1]) + cls = cls.split('.')[-1] + module = __import__(module, fromlist=[module]) + return getattr(module, cls) + + class OrderedSet(collections.MutableSet): def __init__(self, iterable=None): self.end = end = []