Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resubmit cancelled/deleted jobs #29

Merged
merged 5 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@

### `wagtail-localize-smartling` issues

- [ ] Duplicate jobs get created under some circumstances:
- [x] Duplicate jobs get created under some circumstances:
- submitting parent pages including subtree when child pages are already translated
- translating a page, converting it back to an alias, submitting it for translation again
- [ ] It's possible for multiple Smartling jobs to end up with identical content
- [x] It's possible for multiple Smartling jobs to end up with identical content
if translations are submitted/updated more than once in between runs of
`sync_smartling`. This seems to be OK for a single target language. Is it OK for
multiple target languages?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{% extends "wagtailadmin/generic/base.html" %}
{% load i18n %}

{% block main_content %}
<div class="w-mt-6">
<p>
{{ view.confirmation_message }}
{% if view_url %}{{ view_url }}{% endif %}
</p>
<form method="POST">
{% csrf_token %}
<button type="submit" class="button">{% trans 'Yes, resubmit' %}</button>
<a href="{{ view.get_success_url }}" class="button button-secondary">{% trans "No, don't resubmit" %}</a>
</form>
</div>
{% endblock %}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from typing import Any

from django import template
from django.urls import reverse
from django.utils.translation import gettext as _
from wagtail.admin import messages as admin_messages
from wagtail.admin.utils import set_query_params

from wagtail_localize_smartling.constants import UNTRANSLATED_STATUSES
from wagtail_localize_smartling.models import Job
Expand All @@ -23,7 +25,7 @@ def smartling_edit_translation_message(context):
}

translation = context["translation"]
jobs = translation.smartling_jobs.exclude(status__in=UNTRANSLATED_STATUSES)
jobs = translation.smartling_jobs.all()

jobs_exists = bool(jobs)
inclusion_context["show_message"] = jobs_exists
Expand All @@ -35,20 +37,38 @@ def smartling_edit_translation_message(context):
# then pk descending. So we choose the first in the list. This may mean the job
# status will show as unsynced in the message below
latest_job = list(jobs)[0]
message = _(
"This translation is managed by Smartling. Changes made here will be lost the "
"next time translations are imported from Smartling. "
f"Job status: {latest_job.get_status_display()}"
)
buttons = []
if smartling_url := format_smartling_job_url(latest_job):

if latest_job.status in UNTRANSLATED_STATUSES:
message = _(
"The latest Smartling job for this translation "
f"was {latest_job.get_status_display().lower()}."
)
resubmit_url = set_query_params(
reverse("wagtail_localize_smartling_retry_job", args=(latest_job.pk,)),
{"next": context["request"].path},
)
buttons.append(
admin_messages.button(
smartling_url,
_("View job in Smartling"),
resubmit_url,
_("Resubmit to Smartling"),
new_window=True,
),
)
else:
message = _(
"This translation is managed by Smartling. Changes made here will be lost "
"the next time translations are imported from Smartling. "
f"Job status: {latest_job.get_status_display()}"
)
if smartling_url := format_smartling_job_url(latest_job):
buttons.append(
admin_messages.button(
smartling_url,
_("View job in Smartling"),
new_window=True,
),
)

inclusion_context["message"] = admin_messages.render(
message,
Expand Down
94 changes: 92 additions & 2 deletions src/wagtail_localize_smartling/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@

from typing import Any

from django.shortcuts import get_object_or_404, redirect
from django.urls import reverse
from django.utils import timezone
from django.utils.html import format_html
from django.utils.translation import gettext_lazy as _
from django.views.generic import TemplateView
from wagtail.admin.views.generic import WagtailAdminTemplateMixin
from wagtail.admin.auth import permission_denied
from wagtail.admin.utils import get_valid_next_url_from_request
from wagtail.admin.views.generic import (
PermissionCheckedMixin,
WagtailAdminTemplateMixin,
)
from wagtail.models import Locale
from wagtail.permission_policies import ModelPermissionPolicy

from .models import Project
from .constants import UNTRANSLATED_STATUSES
from .models import Job, Project
from .templatetags.wagtail_localize_smartling_admin_tags import smartling_job_url
from .utils import (
format_smartling_project_url,
get_wagtail_source_locale,
Expand Down Expand Up @@ -74,3 +85,82 @@ def get_context_data(self, **kwargs):
context["suggested_source_locale_exists"] = False

return super().get_context_data(**context)


class SmartlingResubmitJobView( # pyright: ignore[reportIncompatibleMethodOverride]
PermissionCheckedMixin, WagtailAdminTemplateMixin, TemplateView
):
_show_breadcrumbs = True
page_title = _("Resubmit to Smartling")
template_name = "wagtail_localize_smartling/admin/resubmit_job.html"
header_icon = "wagtail-localize-language"
object: Job
permission_policy = ModelPermissionPolicy(Job)
permission_required = "view"
jobs_report_url: str = ""

def get_breadcrumbs_items(self):
# TODO: link to the report view
return super().get_breadcrumbs_items() + [
{"url": self.jobs_report_url, "label": _("Smartling jobs")},
{"url": "", "label": self.page_title},
]

def get_object(self):
return get_object_or_404(Job, pk=self.kwargs.get("job_id"))

def dispatch(self, request, *args, **kwargs):
self.object = self.get_object()
if self.object.status not in UNTRANSLATED_STATUSES:
return permission_denied(request)

# Check that the given Job is the latest. We can use any of its translations
translation = self.object.translations.first()
if translation is None:
# should cover the case where the translation was converted back to an alias
return permission_denied(request)

# Jobs are ordered by `first_synced_at`, with null values coming at the top
# then pk descending.
latest_job = translation.smartling_jobs.values_list("pk", flat=True)[0] # pyright: ignore[reportAttributeAccessIssue]
if latest_job != self.object.pk:
return permission_denied(request)

self.jobs_report_url = reverse("wagtail_localize_smartling_jobs:index")
return super().dispatch(request, *args, **kwargs)

def get(self, request, *args, **kwargs):
context = self.get_context_data(object=self.object)
return self.render_to_response(context)

def post(self, request, *args, **kwargs):
if self.object:
due_date = self.object.due_date
Job.get_or_create_from_source_and_translation_data(
self.object.translation_source,
self.object.translations.all(),
user=request.user,
due_date=due_date if due_date and due_date >= timezone.now() else None,
)

return redirect(self.get_success_url())

def get_success_url(self):
return get_valid_next_url_from_request(self.request) or self.jobs_report_url

@property
def confirmation_message(self):
return _("Are you sure you want to resubmit this job?")

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["view"] = self
if smartling_url := smartling_job_url(self.object):
context["view_url"] = format_html(
'<a href="{}" title="Reference {}" target="_blank">{}</a>',
smartling_url,
self.object.reference_number,
_("View job in Smartling"),
)

return context
4 changes: 4 additions & 0 deletions src/wagtail_localize_smartling/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,5 +203,9 @@ class JobViewSet(ModelViewSet):
def permission_policy(self):
return JobPermissionPolicy(self.model)

@cached_property
def url_namespace(self): # # pyright: ignore[reportIncompatibleVariableOverride]
return "wagtail_localize_smartling_jobs"


smartling_job_viewset = JobViewSet("smartling-jobs")
10 changes: 9 additions & 1 deletion src/wagtail_localize_smartling/wagtail_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,20 @@
from wagtail.admin.menu import MenuItem

from . import admin_urls
from .views import SmartlingResubmitJobView
from .viewsets import smartling_job_viewset


@hooks.register("register_admin_urls") # pyright: ignore[reportOptionalCall]
def register_admin_urls():
return [path("smartling/", include(admin_urls))]
return [
path("smartling/", include(admin_urls)),
path(
"smartling-jobs/resubmit/<int:job_id>/",
SmartlingResubmitJobView.as_view(),
name="wagtail_localize_smartling_retry_job",
),
]


@hooks.register("register_settings_menu_item") # pyright: ignore[reportOptionalCall]
Expand Down
20 changes: 20 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import pytest

from django.contrib.auth.models import Permission
from wagtail.coreutils import get_supported_content_language_variant
from wagtail.models import Locale, Page
from wagtail_localize.models import LocaleSynchronization
Expand Down Expand Up @@ -55,6 +56,25 @@ def superuser():
return UserFactory(is_superuser=True)


@pytest.fixture()
def regular_user():
user = UserFactory(is_superuser=False)
user.user_permissions.add(Permission.objects.get(codename="access_admin"))
return user


@pytest.fixture()
def smartling_reporter():
user = UserFactory(is_superuser=False)
user.user_permissions.add(Permission.objects.get(codename="access_admin"))
user.user_permissions.add(
Permission.objects.get(
content_type__app_label="wagtail_localize_smartling", codename="view_job"
)
)
return user


@pytest.fixture()
def smartling_auth(responses):
# Mock API response for authentication
Expand Down
40 changes: 23 additions & 17 deletions tests/translation_components/test_submit_translations.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,29 @@ def test_submitting_for_translation_with_no_existing_jobs_no_child_pages(
assert job.due_date is None


MESSAGE_MANAGED = (
"This translation is managed by Smartling. Changes made here will be "
"lost the next time translations are imported from Smartling. "
"Job status: STATUS_PLACEHOLDER"
)
MESSAGE_RESUBMIT = (
"The latest Smartling job for this translation was STATUS_PLACEHOLDER_LOWER"
)


@pytest.mark.parametrize(
"status,show_message",
"status,message",
[
(JobStatus.UNSYNCED, True),
(JobStatus.AWAITING_AUTHORIZATION, True),
(JobStatus.IN_PROGRESS, True),
(JobStatus.COMPLETED, True),
(JobStatus.CANCELLED, False),
(JobStatus.DELETED, False),
(JobStatus.UNSYNCED, MESSAGE_MANAGED),
(JobStatus.AWAITING_AUTHORIZATION, MESSAGE_MANAGED),
(JobStatus.IN_PROGRESS, MESSAGE_MANAGED),
(JobStatus.COMPLETED, MESSAGE_MANAGED),
(JobStatus.CANCELLED, MESSAGE_RESUBMIT),
(JobStatus.DELETED, MESSAGE_RESUBMIT),
],
)
def test_managed_by_message_on_translation(
client, root_page, superuser, smartling_project, status, show_message
client, root_page, superuser, smartling_project, status, message
):
client.force_login(superuser)

Expand Down Expand Up @@ -175,15 +185,11 @@ def test_managed_by_message_on_translation(
job.save(update_fields=update_fields)

response = client.get(redirect_url)
expected = (
"This translation is managed by Smartling. Changes made here will be "
"lost the next time translations are imported from Smartling. "
f"Job status: {job.get_status_display()}" # pyright: ignore[reportAttributeAccessIssue]
)
if show_message:
assert expected in response.content.decode("utf-8")
else:
assert expected not in response.content.decode("utf-8")
status_display = job.get_status_display() # pyright: ignore[reportAttributeAccessIssue]
expected_message = message.replace(
"STATUS_PLACEHOLDER_LOWER", status_display.lower()
).replace("STATUS_PLACEHOLDER", status_display)
assert expected_message in response.content.decode("utf-8")


# TODO test existing jobs in various states
Expand Down
Loading