Improve workflow around imported translations via tasks and emails #54
Conversation
…t that needs republishing Note that this deliberately doesn't use the Tasks and Workflow approach, because Workflows don't handle Snippets, and also have a cascading behaiviour that auto-triggers for pages under its umbrella. What we want to to is very specifically create a LandedTranslationTask when a translation lands
…anslation is imported
Truncated to a max of 7 tasks. If there are more, we show a link to a custom view listing them all.
… Snippet is published
…rovers group When a Job's translations are imported, meaning that there are now content objects that may need repblishing in order to make the new strings live, we send an email to Translation Approvers about this. The email sending can be turned off via a setting. This changeset also backfills test coverage for all signal handlers and the LandedTranslationTask model and its related Manager.
…ith earlier Wagtails
…Django settings ...and add tests
| ) | ||
| approver_email_addresses = [ | ||
| email for email in approver_email_addresses if email | ||
| ] # Safety check to ensure no empty email addresses are included |
There was a problem hiding this comment.
Curious what do you think about filter?
approver_email_addresses = filter(None, approver_email_addresses)There was a problem hiding this comment.
With the len below you'll need
approver_email_addresses = list(filter(None, approver_email_addresses))unfortunately, otherwise it acts like a list just fine for iterations. So kind of a toss up on which seems cleaner or clearer.
|
|
||
| <div id="landed-translations-content" class="w-panel__content"> | ||
| <div class="help-block help-info"> | ||
| <svg class="icon icon-help icon" aria-hidden="true"><use href="#icon-help"></use></svg> <p> |
There was a problem hiding this comment.
Could move that <p> way over there back with the rest of the party.
| mock_tasks_filter.assert_called_once_with( | ||
| content_type=mock_content_type.return_value, | ||
| object_id=5432, | ||
| ) |
There was a problem hiding this comment.
Could throw an assert that complete is not called?
| if is_a_snippet: | ||
| mock_close_translation_landed_task.assert_called_once_with(mock_instance) | ||
| else: | ||
| assert not mock_close_translation_landed_task.called |
There was a problem hiding this comment.
There's also mock_close_translation_landed_task.assert_not_called() if that feels better
| if fake_a_page: | ||
| mock_close_translation_landed_task.assert_called_once_with(instance) | ||
| else: | ||
| assert not mock_close_translation_landed_task.called |
There was a problem hiding this comment.
Could be used here too, maybe others: assert_not_called()
| publish a particular Page or Snippet, which has just had translations land. | ||
|
|
||
| Note that this is _not_ a subclass of Task, and we don't want it to sit | ||
| within a workflow because Workflows a) don't support Snippets and b) |
There was a problem hiding this comment.
note that snippets can support workflows - https://docs.wagtail.org/en/stable/topics/snippets/features.html#enabling-workflows-for-snippets so this is not entirely true :)
but the other point stands strong
This changeset is focused on making it easier to handle a large volume of translations flowing back from Smartling, all of which will need manually publishing1.
We have two behaviours aimed at helping with this: sending emails and creating shared tasks, both of which affect users who are added to a new "Translation Approvers" Group (which is bootstrapped, but not populated, via a data migration in this changeset.
(Note that this behaviour can be turned off wholesale via the
SEND_EMAIL_ON_TRANSLATION_IMPORTapp-level setting. In the future we may support user-level control of the notification via custom User account settings)There is also a standalone view of the list, which handles any overflow beyond the
settings.MAX_APPROVAL_TASKS_ON_DASHBOARDlimit set (default:" 7)Resolves #44
Footnotes
unless it's keeping an existing page in sync, which does support auto-publishing ↩