diff --git a/app/admin_ui/tables/tables.py b/app/admin_ui/tables/tables.py index a7a2afc7..8534c786 100644 --- a/app/admin_ui/tables/tables.py +++ b/app/admin_ui/tables/tables.py @@ -505,8 +505,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.uuid, - # this property is coming from an annotation on the inital query - "draft_uuid": record.draft_uuid, "model": camel_to_snake(record.model_name), }, ), @@ -526,7 +524,13 @@ class CampaignChangeListTable(LimitedTableBase): verbose_name="Short Name", accessor="latest_update__short_name", backup_accessor="content_object.short_name", - linkify=("change-update", [tables.A("uuid")]), + linkify=( + "canonical-redirect", + { + "canonical_uuid": tables.A('uuid'), + "model": 'campaign', + }, + ), ) funding_agency = BackupValueColumn( verbose_name="Funding Agency", @@ -548,8 +552,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.uuid, - # this property is coming from an annotation on the inital query - "draft_uuid": record.draft_uuid, "model": camel_to_snake(record.model_name), }, ), @@ -578,8 +580,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.uuid, - # this property is coming from an annotation on the inital query - "draft_uuid": record.draft_uuid, "model": camel_to_snake(record.model_name), }, ), @@ -600,8 +600,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.uuid, - # this property is coming from an annotation on the inital query - "draft_uuid": record.draft_uuid, "model": camel_to_snake(record.model_name), }, ), @@ -633,8 +631,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.model_instance_uuid or record.uuid, - # TODO this change object has not property draft uuid - "draft_uuid": record.uuid, "model": camel_to_snake(record.model_name), }, ), @@ -909,8 +905,6 @@ def render_short_name(self, value, record): 'canonical-redirect', kwargs={ "canonical_uuid": record.uuid, - # this property is coming from an annotation on the inital query - "draft_uuid": record.draft_uuid, "model": camel_to_snake(record.model_name), }, ), diff --git a/app/admin_ui/templates/snippets/object_header_tabs.html b/app/admin_ui/templates/snippets/object_header_tabs.html index c7d35145..5b418a49 100644 --- a/app/admin_ui/templates/snippets/object_header_tabs.html +++ b/app/admin_ui/templates/snippets/object_header_tabs.html @@ -7,7 +7,7 @@
  • Published @@ -15,7 +15,7 @@
  • Edits {% if draft_status == 'Published' and url_name == 'create-update' %} diff --git a/app/admin_ui/templatetags/template_extras.py b/app/admin_ui/templatetags/template_extras.py index 81e4d506..676123d7 100644 --- a/app/admin_ui/templatetags/template_extras.py +++ b/app/admin_ui/templatetags/template_extras.py @@ -94,7 +94,6 @@ def object_header_tabs(context, change: Change, canonical_change: Optional[Chang "draft_status": draft_status, "draft_status_class": draft_status_class, "canonical_uuid": canonical_uuid, - "draft_uuid": change.uuid, "has_progress_draft": has_progress_draft, "has_published_draft": has_published_draft, "request": context.get("request"), diff --git a/app/admin_ui/urls.py b/app/admin_ui/urls.py index daf98948..aba89fab 100644 --- a/app/admin_ui/urls.py +++ b/app/admin_ui/urls.py @@ -58,19 +58,19 @@ path('v2/', v2.CanonicalRecordList.as_view(), name="canonical-list"), # Helper route to redirect user to appropriate view without prior knowledge of record's status (ie if it's been published). If published, return redirect to `//published`. Otherwise, redirect to `//edit`. path( - 'v2///', + 'v2//', v2.redirect_helper, name="canonical-redirect", ), # Read-only view the published record for this concept. Render `400` response if record is not yet published. path( - 'v2///published/', + 'v2///published', v2.CanonicalRecordPublished.as_view(), name="canonical-published-detail", ), - # Update for latest published draft. + # View latest draft for canonical record. path( - 'v2///edit/', + 'v2///edit', v2.CanonicalDraftEdit.as_view(), name="canonical-draft-edit", ), diff --git a/app/admin_ui/views/change.py b/app/admin_ui/views/change.py index 7295715e..68042fb5 100644 --- a/app/admin_ui/views/change.py +++ b/app/admin_ui/views/change.py @@ -90,7 +90,6 @@ def get_queryset(self): Change.objects.of_type(*self.models) .filter(action=Change.Actions.CREATE) .annotate( - draft_uuid=Subquery(related_drafts.values("uuid")[:1]), latest_status=Subquery(related_drafts.values("status")[:1]), latest_action=Subquery(related_drafts.values("action")[:1]), latest_updated_at=Subquery(related_drafts.values("updated_at")[:1]), @@ -480,19 +479,25 @@ def form_valid(self, form): mark_safe(f"Unable to transition draft. {format_validation_error(err)}"), ) else: - obj = self.get_object() messages.success( self.request, ( - f"Transitioned \"{obj.model_name}: {obj.update.get('short_name', obj.uuid)}\" " - f"to \"{obj.get_status_display()}\"." + f"Transitioned {form.change.model_name}:" + + f" {form.change.update.get('short_name', form.change.uuid)!r}" + + f" to {form.change.get_status_display()!r}." ), ) return super().form_valid(form) def get_success_url(self): - return self.request.META.get("HTTP_REFERER") or super().get_success_url() + return reverse( + "canonical-redirect", + kwargs={ + "canonical_uuid": self.kwargs[self.pk_url_kwarg], + "model": self.get_form().change.content_type.model, + }, + ) def format_validation_error(err: ValidationError) -> str: diff --git a/app/admin_ui/views/v2.py b/app/admin_ui/views/v2.py index e3c4b12d..d0c69d78 100644 --- a/app/admin_ui/views/v2.py +++ b/app/admin_ui/views/v2.py @@ -3,18 +3,18 @@ from django.contrib.auth.decorators import login_required from django.utils.decorators import method_decorator from django.views.generic.detail import DetailView -from django_filters.views import FilterView -from django_tables2.views import SingleTableMixin, SingleTableView from django.forms import modelform_factory from django.http import Http404, HttpResponseBadRequest from django.shortcuts import redirect from django.urls import reverse from django.contrib.contenttypes.models import ContentType from django.views.generic.edit import UpdateView -from django.db.models import Q, OuterRef, Subquery +from django.db.models import OuterRef, Subquery from django.views.generic.edit import CreateView -from admin_ui.config import MODEL_CONFIG_MAP +from django_filters.views import FilterView +from django_tables2 import SingleTableView, SingleTableMixin +from admin_ui.config import MODEL_CONFIG_MAP from admin_ui.views.published import ModelObjectView from api_app.models import Change @@ -36,36 +36,33 @@ from ..tables import DraftHistoryTable -# TODO add login requirement -def redirect_helper(request, canonical_uuid, draft_uuid, model): - try: - draft = Change.objects.get(uuid=canonical_uuid) - has_progress_draft = ( - Change.objects.exclude(status=Change.Statuses.PUBLISHED) - .filter(Q(uuid=draft.uuid) | Q(model_instance_uuid=draft.uuid)) - .exists() - ) - if draft.status == Change.Statuses.PUBLISHED and not has_progress_draft: - return redirect( - reverse( - "canonical-published-detail", - kwargs={ - "canonical_uuid": canonical_uuid, - "model": model, - "draft_uuid": draft_uuid, - }, - ) - ) - # TODO return redirect to edit view - # return HttpResponse("Todo return redirect") - return redirect( +@login_required +def redirect_helper(request, canonical_uuid, model): + """ + Redirect to the latest draft edit if non-publishd edit exist. Otherwise, send to + published detail view. + """ + return ( + redirect( reverse( "canonical-draft-edit", - kwargs={"canonical_uuid": canonical_uuid, "model": model, "draft_uuid": draft_uuid}, + kwargs={ + "canonical_uuid": canonical_uuid, + "model": model, + }, + ) + ) + if Change.objects.related_in_progress_drafts(uuid=canonical_uuid).exists() + else redirect( + reverse( + "canonical-published-detail", + kwargs={ + "canonical_uuid": canonical_uuid, + "model": model, + }, ) ) - except Change.DoesNotExist: - raise Http404("Canonial UI does not exist") + ) # Lists all the canonical records for a given model type @@ -86,9 +83,9 @@ def get_queryset(self): However, we want to display the most recent related draft in the table. """ # find the most recent drafts for each canonical CREATED draft - related_drafts = Change.objects.filter( - Q(model_instance_uuid=OuterRef("uuid")) | Q(uuid=OuterRef("uuid")) - ).order_by("status", "-updated_at") + related_drafts = Change.objects.related_drafts(OuterRef("uuid")).order_by( + "status", "-updated_at" + ) latest_published_draft = Change.objects.filter( status=Change.Statuses.PUBLISHED, @@ -102,7 +99,6 @@ def get_queryset(self): ) ) .annotate( - draft_uuid=Subquery(related_drafts.values("uuid")[:1]), latest_status=Subquery(related_drafts.values("status")[:1]), latest_action=Subquery(related_drafts.values("action")[:1]), latest_updated_at=Subquery(related_drafts.values("updated_at")[:1]), @@ -135,10 +131,7 @@ class ChangeHistoryList(SingleTableView): template_name = "api_app/canonical/change_history.html" def get_queryset(self): - return Change.objects.filter( - Q(model_instance_uuid=self.kwargs[self.pk_url_kwarg]) - | Q(uuid=self.kwargs[self.pk_url_kwarg]) - ) + return Change.objects.related_drafts(self.kwargs[self.pk_url_kwarg]) def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -176,13 +169,21 @@ def get_context_data(self, **kwargs): @method_decorator(login_required, name="dispatch") class CanonicalRecordPublished(ModelObjectView): + """ + Render a read-only form displaying the latest published draft. + """ + model = Change pk_url_kwarg = 'canonical_uuid' template_name = "api_app/canonical/published_detail.html" fields = ["content_type", "model_instance_uuid", "action", "update"] def get_model_form_content_type(self) -> ContentType: - return Change.objects.get(uuid=self.kwargs[self.pk_url_kwarg]).content_type + return ( + Change.objects.prefetch_related('content_type') + .get(uuid=self.kwargs[self.pk_url_kwarg]) + .content_type + ) def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -193,20 +194,18 @@ def get_context_data(self, **kwargs): Change.objects.get(uuid=self.kwargs[self.pk_url_kwarg]).model_name.lower() ), "display_name": Change.objects.get(uuid=self.kwargs[self.pk_url_kwarg]).model_name, - "has_progress_draft": ( - Change.objects.exclude(status=Change.Statuses.PUBLISHED) - .filter( - Q(uuid=self.kwargs[self.pk_url_kwarg]) - | Q(model_instance_uuid=self.kwargs[self.pk_url_kwarg]) - ) - .exists() - ), + "has_progress_draft": Change.objects.related_in_progress_drafts( + self.kwargs[self.pk_url_kwarg] + ).exists(), } @method_decorator(login_required, name="dispatch") class CanonicalDraftEdit(NotificationSidebar, mixins.ChangeModelFormMixin, UpdateView): - """This view is in charge of editing existing drafts. + """ + This view is in charge of editing the latest in-progress drafts. If no in-progress + drafts are available, a 404 is returned. + If the draft has a related published record, a diff view is returned to the user. Otherwise a single form view is returned. """ @@ -215,28 +214,22 @@ class CanonicalDraftEdit(NotificationSidebar, mixins.ChangeModelFormMixin, Updat prefix = "change" template_name = "api_app/canonical/change_update.html" pk_url_kwarg = 'canonical_uuid' + queryset = Change.objects.all().exclude(status=Change.Statuses.PUBLISHED) def get_queryset(self): return Change.objects.all() def get(self, request, *args, **kwargs): self.object = self.get_object() - has_progress_draft = ( - Change.objects.exclude(status=Change.Statuses.PUBLISHED) - .filter( - model_instance_uuid=self.canonical_change.uuid, - content_type=self.canonical_change.content_type, - action=Change.Actions.UPDATE, - ) - .exists() + in_progress_drafts = Change.objects.related_in_progress_drafts( + self.kwargs["canonical_uuid"] ) - has_published_draft = Change.objects.filter( - Q(uuid=self.canonical_change.uuid) | Q(model_instance_uuid=self.canonical_change.uuid), - status=Change.Statuses.PUBLISHED, - ).exists() + published_drafts = Change.objects.related_drafts(self.kwargs["canonical_uuid"]).filter( + status=Change.Statuses.PUBLISHED + ) - if not has_progress_draft and has_published_draft: + if not in_progress_drafts.exists() and published_drafts.exists(): return redirect( reverse( "canonical-published-detail", @@ -251,48 +244,19 @@ def get(self, request, *args, **kwargs): def get_object(self, queryset=None): if not queryset: - queryset = self.get_queryset() - self.canonical_change = queryset.get(uuid=self.kwargs["canonical_uuid"]) - # if the canonical record is not published, return the record itself - if self.canonical_change.status != Change.Statuses.PUBLISHED: - return self.canonical_change + queryset = self.queryset - # if canonical record is published, return the record where the model_instance_uuid equals our canonical_uuid + canonical_uuid = self.kwargs["canonical_uuid"] - # TODO include url to make this check - # if there is no update in progress return most recently published object - if ( - not Change.objects.exclude(status=Change.Statuses.PUBLISHED) - .filter( - model_instance_uuid=self.canonical_change.uuid, - content_type=self.canonical_change.content_type, - action=Change.Actions.UPDATE, - ) - .exists() - ): - return ( - Change.objects.filter( - status=Change.Statuses.PUBLISHED, - model_instance_uuid=self.kwargs["canonical_uuid"], - ) - .order_by("updated_at") - .last() - ) - - # try: - # TODO: include uuid in url - return Change.objects.exclude(status=Change.Statuses.PUBLISHED).get( - model_instance_uuid=self.canonical_change.uuid, - content_type=self.canonical_change.content_type, - action=Change.Actions.UPDATE, - ) + if obj := queryset.related_drafts(canonical_uuid).order_by('status').first(): + return obj + raise Http404(f'No in progress draft with Canonical UUID {canonical_uuid!r}') def get_success_url(self, **kwargs): url = reverse( "canonical-redirect", kwargs={ "canonical_uuid": self.kwargs[self.pk_url_kwarg], - "draft_uuid": self.object.pk, "model": self.kwargs["model"], }, ) @@ -313,9 +277,7 @@ def get_context_data(self, **kwargs): "ancestors": context["object"].get_ancestors().select_related("content_type"), "descendents": context["object"].get_descendents().select_related("content_type"), "comparison_form": self._get_comparison_form(context['model_form']), - "canonical_object": self.canonical_change, "canonical_uuid": self.kwargs[self.pk_url_kwarg], - "draft_uuid": self.object.pk, } def _get_comparison_form(self, model_form): @@ -415,7 +377,6 @@ def get_success_url(self): kwargs={ "canonical_uuid": self.object.pk, "model": self._model_name, - "draft_uuid": self.object.pk, }, ) @@ -470,7 +431,6 @@ def get_context_data(self, **kwargs): Change.objects.get(uuid=self.kwargs[self.pk_url_kwarg]).model_name.lower() ), "display_name": Change.objects.get(uuid=self.kwargs[self.pk_url_kwarg]).model_name, - "draft_uuid": self.object.pk, } def get_success_url(self): @@ -479,8 +439,6 @@ def get_success_url(self): kwargs={ "model": self._model_name, "canonical_uuid": self.kwargs["canonical_uuid"], - # TODO: Check if this is really the right draft_UUID - "draft_uuid": self.object.pk, }, ) diff --git a/app/api_app/models.py b/app/api_app/models.py index 719d02c8..bcb492c0 100644 --- a/app/api_app/models.py +++ b/app/api_app/models.py @@ -6,7 +6,7 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.db import models -from django.db.models import expressions, functions, Subquery +from django.db.models import expressions, functions, Subquery, Q from django.db.models.fields.json import KeyTextTransform from django.db.models.signals import post_save from django.dispatch import receiver @@ -117,6 +117,12 @@ def get_action_display_past_tense(self): class ChangeQuerySet(models.QuerySet): + def related_drafts(self, uuid: str): + return self.filter(Q(uuid=uuid) | Q(model_instance_uuid=uuid)) + + def related_in_progress_drafts(self, uuid: str): + return self.related_drafts(uuid=uuid).exclude(status=Change.Statuses.PUBLISHED) + def of_type(self, *models): """ Limit changes to only those targeted to provided models