This is a follow-up in response to #1188388-108: Entity translation UI in core comment 105 and comment 108

Problem

Currently in the D7 version of ET we have the capability of marking each translation as individually published or not and providing additional translation metadata. We may want to retain this feature also in D8.

Proposed resolution

We will add this metadata to any entity as additional multilingual properties. At the moment only nodes natively provide this kind of information, which will be multilingual as soon as #1498674: Refactor node properties to multilingual. The idea is that nodes override the default translation metadata values with their native multilingual metadata. In a follow-up we could explore the possibility of extending these metadata to any entity regardless of translation.

Remaining tasks

  • Define which strategy we want to implement.
  • @catch requests more reviews
  • Reach the RTBC status.

User interface changes

UI changes need to be described.

API changes

Are there API changes/additions that would affect module, install profile, and theme developers?

Background/Original Issue

cut and pasted from et ui part 2 issue summary to preserve knowledge:
No matter what we decide for the previous point, we should consider whether we should make the publication status a property of the entity translations, and thus making it available for any entity type. Should we only map per-language statuses to entity translation statuses for entities that support that (in core only nodes) or remove the special-casing from nodes and make the status a generally available property, that any entity type can leverage, at least when dealing with translation?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

Title: Make translation status entity a generic property and not a special case of nodes [Follow-up to Entity Translation UI in core] » Make status a generic entity translation property
Component: language system » translation_entity.module
Issue tags: +API clean-up, +feature freeze
plach’s picture

Issue summary: View changes

Updated issue summary. correct the direct link to comment 108

plach’s picture

Issue summary: View changes

Updated issue summary

plach’s picture

Issue summary: View changes

Updated issue.

YesCT’s picture

This one is tagged with feature freeze but has no patch yet. @plach, did you have something started, or should anyone jump in?

plach’s picture

I'm planning to post a patch for this during the upcoming weekend. @bforchhammer will help us with code reviews and coding, if needed.

plach’s picture

Title: Make status a generic entity translation property » Make status and authoring information generic entity translation metatdata
Assigned: Unassigned » plach
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
22.42 KB

The attached patch takes care of the issue (it's a port of the D7 code) and adds also support for authoring information in addition to the translation status. In core nodes are the only entity type already supporting these properties hence in this case the property values are inherited from the node ones and are not exposed on the entity form.

Tests missing.

plach’s picture

FileSize
23.09 KB

Fixed a couple of issues with the previous patch.

plach’s picture

Title: Make status and authoring information generic entity translation metatdata » Add status and authoring information as generic entity translation metatdata

Better title

attiks’s picture

This looks good to me, leaving at NR so we can get some more feedback.

YesCT’s picture

+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationController.phpundefined
@@ -224,9 +224,35 @@ public function entityFormAlter(array &$form, array &$form_state, EntityInterfac
+      $description = $enabled ?
+        t('An unpublished translation will not be visible without translation permissions.') :
+        t('Only this translation is published. You must publish at least one more translation to unpublish this one.');

why not just make this an if?

plach’s picture

I find it more readable this way.

bforchhammer’s picture

Status: Needs review » Needs work

Haven't tested it properly yet, but looks good overall. A few comments...

+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationController.php
@@ -64,7 +64,7 @@ public function retranslate(EntityInterface $entity, $langcode = NULL) {
-      $entity->retranslate[$langcode] = $langcode != $updated_langcode;
+      $entity->translation[$langcode]['translate'] = $langcode != $updated_langcode;

Can we make the variable name (array key) more clear by leaving the "re" prefix in the name of the array key as in 'retranslate', or possibly rename it to 'outdated'?

+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationController.php
@@ -224,9 +224,35 @@ public function entityFormAlter(array &$form, array &$form_state, EntityInterfac
+      $enabled = !$status;
+      if (!empty($status)) {
+        // A new translation is not available in the translation metadata, hence
+        // it should count as one more.
+        $published = $new_translation;
+        foreach ($entity->translation as $langcode => $translation) {
+          $published += $translation['status'];
+        }
+        $enabled = $published > 1;
+      }

I think this is equivalent to:

$enabled = TRUE;
if($status) {
  // ...
  $enabled = $published > 1;
}
+++ b/core/modules/translation_entity/translation_entity.module
@@ -234,6 +234,22 @@ function translation_entity_translate_access(EntityInterface $entity) {
+function translation_entity_view_access(EntityInterface $entity, $langcode, $account = NULL) {
+  $entity_type = $entity->entityType();
+  return !empty($entity->translation[$langcode]['status']) || user_access('translate any entity', $account) || user_access("translate $entity_type entities", $account);
+}

I'm guessing this will change once #1807776: Support both simple and editorial workflows for translating entities is done? (Add edit permissions? Move logic into getTranslationAccess($entity, $op)?)

+++ b/core/modules/translation_entity/translation_entity.module
@@ -453,6 +469,35 @@ function translation_entity_form_alter(array &$form, array &$form_state) {
+    $entity = clone($entity);

This may be a stupid question, but why are we cloning the entity here?

+++ b/core/modules/translation_entity/translation_entity.module
@@ -505,16 +552,32 @@ function translation_entity_entity_insert(EntityInterface $entity) {
+    // Reorder values to match the schema.
+    $values = array();
+    foreach ($fields as $field_name) {
+      $value = is_bool($translation[$field_name]) ? intval($translation[$field_name]) : $translation[$field_name];
+      $values[$field_name] = $value;
+    }

This seems to be a "type conversion" of values from boolean to integer (not a "reordering"). Is this really necessary? I would have expected the DB layer to take care of simple type conversions :)

plach’s picture

Status: Needs work » Needs review
FileSize
36.46 KB
20.77 KB

Thanks! This should take care of #10 and add test coverage:

c84ea7f Issue #1807800: Renamed 'translate' to 'outdated'.
d1a5fc7 Issue #1807800: Improved readability.
7ed774a Issue #1807800: Added test coverage.
I'm guessing this will change once #1807776: Support both simple and editorial workflows for translating entities is done?

Yep, totally.

This may be a stupid question, but why are we cloning the entity here?

Because we are unsetting field values to trigger language fallback. We don't want them to be lost if the entity is saved afterwards :)

This seems to be a "type conversion" of values from boolean to integer (not a "reordering"). Is this really necessary? I would have expected the DB layer to take care of simple type conversions :)

Yes, it's necessary: without it you'll get DB exceptions all the time. It's actually a reordering because the values are inserted into the array in the order expected by the schema.

bforchhammer’s picture

I just tested this again and this looks pretty good. RTBC as far as I'm concerned :)

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTranslationUITest.php
@@ -56,7 +56,58 @@ protected function setupBundle() {
+    // Unpublish the node and check that the trnalstions are unpublished

typo

+++ b/core/modules/translation_entity/translation_entity.pages.inc
@@ -255,8 +253,10 @@ function translation_entity_delete_confirm_submit(array $form, array &$form_stat
-    path_delete(array('source' => $controller->getViewPath($entity), 'langcode' => $language->langcode));
+    $conditions = array('source' => $controller->getViewPath($entity), 'langcode' => $language->langcode);
+    drupal_container()->get('path.crud')->delete($conditions);

I stumbled on that error when testing some other patch and created a new issue for it: #1852394: Fatal error: Call to undefined function path_delete().

plach’s picture

FileSize
36.46 KB

Typo fixed. Anyone daring to RTBC this?

Status: Needs review » Needs work

The last submitted patch, et-metadata-1807800-13.patch, failed testing.

bforchhammer’s picture

Here we go.

Short summary: patch adds two new translation properties: publication status and translation author 4 translation properties: status, author, creation date and modification date. For nodes, these inherit from the respective existing node properties. All properties are covered well in tests. :)

plach’s picture

Status: Needs work » Reviewed & tested by the community

#13: et-metadata-1807800-13.patch queued for re-testing.

plach’s picture

Actually 4 translation properties: status, author, creation date and modification date :)

plach’s picture

Issue tags: -Needs tests
catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationController.phpundefined
@@ -224,9 +224,35 @@ public function entityFormAlter(array &$form, array &$form_state, EntityInterfac
+        t('Only this translation is published. You must publish at least one more translation to unpublish this one.');

The description here is a bit confusing. If you're in the translation UI and you really wanted to unpublish the entire node in the end, how do you get from here to there assuming you have the permissions?

+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationController.phpundefined
@@ -242,6 +268,25 @@ public function entityFormAlter(array &$form, array &$form_state, EntityInterfac
+      $form['translation_entity']['name'] = array(
+        '#type' => 'textfield',
+        '#title' => t('Authored by'),
+        '#maxlength' => 60,
+        '#autocomplete_path' => 'user/autocomplete',
+        '#default_value' => $name,
+        '#description' => t('Leave blank for %anonymous.', array('%anonymous' => variable_get('anonymous', t('Anonymous')))),
+      );
+
+      $date = $new_translation ? REQUEST_TIME : $entity->translation[$form_langcode]['created'];
+      $form['translation_entity']['created'] = array(
+        '#type' => 'textfield',
+        '#title' => t('Authored on'),
+        '#maxlength' => 25,
+        '#description' => t('Format: %time. The date format is YYYY-MM-DD and %timezone is the time zone offset from UTC. Leave blank to use the time of form submission.', array('%time' => format_date($date, 'custom', 'Y-m-d H:i:s O'), '%timezone' => format_date($date, 'custom', 'O'))),
+        '#default_value' => $new_translation ? '' : format_date($date, 'custom', 'Y-m-d H:i:s O'),
+      );

I'm a bit confused about how this gets saved. Isn't it blocked on #1498674: Refactor node properties to multilingual?

+++ b/core/modules/translation_entity/translation_entity.installundefined
@@ -40,12 +40,36 @@ function translation_entity_schema() {
+      'uid' => array(
+        'description' => 'The author of this translation.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,
+      ),
+      'status' => array(
+        'description' => 'Boolean indicating whether the translation is visible to non-translators.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 1,
+      ),
+      'created' => array(
+        'description' => 'The Unix timestamp when the translation was created.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,
+      ),
+      'changed' => array(
+        'description' => 'The Unix timestamp when the translation was most recently saved.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,
+      ),

OK so this is where it gets saved without the multilingual patch, but this seems a bit confusing - won't we end up with a field for 'author' and another field for 'translation author' then?

plach’s picture

@catch:

This feedback makes me think the issue summary could use some improvement :)

What I'm proposing is that we provide these additional translation metadata to support editorial workflows for translation. At the moment only nodes natively provide this kind of information, which will be multilingual as soon as #1498674: Refactor node properties to multilingual is done. The idea is that nodes override the default translation metadata values with their native multilingual metadata. In a follow-up we could explore the possibility of extending these metadata to any entity regardless of translation.

The description here is a bit confusing. If you're in the translation UI and you really wanted to unpublish the entire node in the end, how do you get from here to there assuming you have the permissions?

For nodes there will be no way to "unpublish" the node altogether, it will have a per-language status. AAMOF this part of the UI is hidden for nodes, which only have the form elements for the native metadata.

I'm a bit confused about how this gets saved. Isn't it blocked on #1498674: Refactor node properties to multilingual?
OK so this is where it gets saved without the multilingual patch, but this seems a bit confusing - won't we end up with a field for 'author' and another field for 'translation author' then?

At the moment this behavior is semibroken, because the translation metadata for nodes is actually stored in a multilingual fashion, but it is overridden with the native metadata which is not multilingual yet, hence it is reset each time the node is saved.

With the upcoming feature freeze I thought this was a reasonable approach, since the ML nodes patch is still blocked on other refactorings, however now we might want to postpone this to wait to get a correct behavior. OTOH the code here should start working correctly without any other required change as soon as the ML nodes patch is committed, so we may want to go on and commit this.

I don't know what kind of work is needed here. Could you clarify, please?

YesCT’s picture

YesCT’s picture

Issue summary: View changes

Updated issue summary preserve et ui part 2 issue summary knowledge

plach’s picture

Status: Needs work » Reviewed & tested by the community

Setting back to RTBC as there is a (remote) possibility that #20 actually solved the concerns raised by @catch. This is just to ensure the issue doesn't get out of his radar.

I'm totally available to rework anything requiring it but I need more information.

catch’s picture

Status: Reviewed & tested by the community » Needs review

OK I still don't get it.

Let's just take the uid field:

+      'uid' => array(
+        'description' => 'The author of this translation.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,
+      ),

Is this:

1. {node}.uid but allowing a translation to refer to a different author?

2. The uid of the person who actually added this translation?

Similar problem to #1528028: Add tests for reverting revisions where revision_uid and uid differ where the column is ambiguous.

plach’s picture

In the current proposal {node}.uid (let's suppose this is already ML) and {translation_entity}.uid hold the same (duplicated) information, meaning that the translation they refer to has been authored by {user}.uid.

Any other entity will have this information stored only in the {translation_entity} table.

This implies that the UI for any entity except nodes will have the form elements letting the user enter the translation author, while nodes will have only their usual author widget, just varying by language.

plach’s picture

Status: Needs review » Reviewed & tested by the community

Trying again :)

catch’s picture

What happens if you want to track the translation authors for nodes in addition to translating the author?

plach’s picture

In this case we are storing multilingual user references, I am not sure I get what you mean with "translating the author" :)

plach’s picture

Let me add that there is already a planned follow-up (#1810370: Entity Translation API improvements) that might turn the data we are adding here into true multilingual properties of the entity they are attached to. The alternative plan, which is the one I like more right now, is making the records in the translation_entity table actual (non-translatable) entities. This would help a lot with per-language hooks and single-entity access control.

catch’s picture

Status: Reviewed & tested by the community » Needs review

It means that in one case it's a "multilingual use reference", and in another case it's "metadata about the author of a translation". I still think it's too odd that exactly the same information means two different things. Marking CNR again in the hope of more reviews.

plach’s picture

I don't think they mean two different things: they both are metadata about the author of the content in the particular language they concern, on the contrary I see them as exactly the same information. However I'll ask Gabor to chime-in here.

Gábor Hojtsy’s picture

@catch: once the node property multilingual issue is resolved, we could track authors per language. Those would be the node authors, not the revision authors to relate to the similar issue you cited. (The node.uid is neither the user who created the node, it is the user who the creator wanted to credit the node to, right? :) So the translation uid has the same meaning as the node uid, it is the translator user who was credited for this translation.

I did not look at the internals of this patch, but @plach noted above that once that node issue is resolved, this patch would have no effect on the behaviour there and simulate that behaviour for other entities. Let's take taxonomy terms. Those do not have status or authors, so if we'd keep it like it is, their translations would equally be "published" right away. This issue attempts to solve that problem by introducing status for translations only.

This looks like generally useful for entities which do not have status by themselves. In core, I think that is taxonomy terms and vocabularies only, and vocabularies are going away from being content entities. This could however apply to other contrib entity types too. An alternative is to say all entities are required to provide status and status should be a multilingual property. Another alternative is to say those entities that in themselves do not support publication status should not have support for translation workflows where the translation might be unpublished or hack around the system in contrib some other way. That would keep the separate handling of status-enabled and status-less entities on the translation level.

Hope this helps.

Gábor Hojtsy’s picture

Issue summary: View changes

Updated issue summary

plach’s picture

@catch:

It would really help if we could get this committed or marked back 'needs work' with some indication on the things to fix.

plach’s picture

FileSize
36.46 KB

rerolled

Gábor Hojtsy’s picture

@catch: any updated feedback based on the explanation would be very welcome! Thanks a ton!

Gábor Hojtsy’s picture

Issue tags: +sprint

Also bringing on D8MI sprint board.

YesCT’s picture

Status: Needs review » Needs work
Issue tags: +translatable fields, +API clean-up, +D8MI, +sprint, +language-content, +feature freeze

The last submitted patch, et-metadata-1807800-33.patch, failed testing.

catch’s picture

Assigned: plach » fago

An alternative is to say all entities are required to provide status and status should be a multilingual property. Another alternative is to say those entities that in themselves do not support publication status should not have support for translation workflows where the translation might be unpublished or hack around the system in contrib some other way.

Right, one of these is along the lines I was thinking, but I'm not sure which is the best option, however since this confuses me I'd not be surprised if it confuses others too.

I'd really like to get a review from someone here who's 1. not me, 2. not plach or Gabor, just to see if it's just me who thinks this is odd. Assigning to fago for a bit but doesn't have to be fago exclusively.

plach’s picture

Status: Needs work » Needs review
Issue tags: -translatable fields, -API clean-up, -D8MI, -sprint, -language-content, -feature freeze

#33: et-metadata-1807800-33.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +translatable fields, +API clean-up, +D8MI, +sprint, +language-content, +feature freeze

The last submitted patch, et-metadata-1807800-33.patch, failed testing.

fago’s picture

It means that in one case it's a "multilingual use reference", and in another case it's "metadata about the author of a translation". I still think it's too odd that exactly the same information means two different things. Marking CNR again in the hope of more reviews.

hm, I think that this is ok. Having a "multilingual user reference" just says that you store a user reference per language - the meaning we put into it is left to us. Thus, in the multi-lingual node author case we would have the meaning of "the author of the node in a certain language". That makes sense to me.

An alternative is to say all entities are required to provide status and status should be a multilingual property. Another alternative is to say those entities that in themselves do not support publication status should not have support for translation workflows where the translation might be unpublished or hack around the system in contrib some other way.

Yes, this is an interesting question to figure out. Personally I think that this kind of editing metadata should not be stored inside the entity, but outside of the edited entities. I'd only store it with the entity if it's to be considered as part of the entity, e.g. to a node ("document") the author and the creation date is clearly crucial information and part of the entity although it can be considered metadata. It's more than the CMS having metadata of who created/edited when/what. That this information is displayed with nodes is a good indicator for the importance of that information and I agree with Gabor that it's a valid and important use case to be able to specify custom information for that (what might differ with the reality). Thus, an editor could go and post an article specifying a custom creation and author information while the CMS should still track who really created/edited when/what.

Thus, who created/edited when/what is metadata we want to keep generally, thus should be stored in its own (simple) non-translatable entity as plach suggests.

But then, I see two use-case this entity should fullfill:
a) A simple way to lookup editing metadata, e.g. when was this term last edited?
b) Provide a log of who edited when/what.

For a) I think we should have a similar table as created by this patch, but as its own simple (untranslatable, unrevisionable?) entity as plach suggests. We can make looking-up this information easy by e.g. adding a computed-field, so you could access it via $entity->metadata->created or so, while the editing-metadata entity gets lazy-loaded.

b) reminds me a bit of message module: A separate logging entity with customizable storage-fields and an easy way to generate a human-readable version of the log. While it would be nice to have something extensible like the message module API inside core, it's probably to much to aim for now. A more straight-forward (but unfortunately not extensible) alternative could be just using revisions on the editing-metadata entity.

However, what imho does not 100% fit into the editing-metadata concept is translation metadata like "is the translation published" or "is it outdated"?
First off, imo the entity in default revision should only contain up2date information - having an outdated value in there would be weird as the API does not support the concept of having outdated values - so developers would not care.
Thus, if the translation is so out of date such that it shouldn't be used/displayed anymore, it should be removed in the revision which adds new information that out-dates the translation. Still, another revision can be prepared that comes with a new updated translation for the given language - with our possibility to track non-default forward revisions this should be solvable in contrib.
If the translation still should be shown/used, but needs another check due to the update - then this is information specific to the translation workflow. Thus, it shouldn't be part of the general editing-metadata by default, but information that the translation workflow should care to track ( - it could still extend the editing-metadata entity type?) That way a contrib translation workflow module could do something completely different.

Summary of my 2 cents:
- We should move editing metadata to its own entity. (We already talked about that at the wscci web service format sprint in Paris last May).
- Imo publishing status is something different and should be handled by entity type, and/or added by contribs. It makes sense to track a publishing status for nodes, but it doesn't for e.g. flag entities or message log entities (greetings from the message module). Probably it makes sense to keep that stored with the entity.
- Translation workflow metadata is different to general editing metadata and differs by the translation workflow used. Storage should be up to the translation module in use.

Gábor Hojtsy’s picture

Title: Add status and authoring information as generic entity translation metatdata » Add status and authoring information as generic entity translation metadata

@fago: sounds like a conceptually sound solution however it is a significant development effort. Do you think this is solvable in the remaining month before feature freeze? Would be good to see cross-opinions from others :) @catch? :)

fago’s picture

@fago: sounds like a conceptually sound solution however it is a significant development effort. Do you think this is solvable in the remaining month before feature freeze?

I think so, however I won't be able to push it myself as I'll be busy with remaining NG-conversions for a while :( Maybe plach or someone else would like to take over the lead here?

YesCT’s picture

Do we want to open a new issue for that, and postpone this to see if we can get it done the new way?

Or... update the issue summary and remaining tasks to clarify we are taking a new approach here. (since we have a limited number of fagos and plachs :) , any clarification and detail in next steps will help)

plach’s picture

Honestly I'm a bit confused too now, so I'd like to rehash all of this to set a clear roadmap taking deadlines into account. I'll get in touch ASAP.

YesCT’s picture

Issue tags: +budapest2012

this was mentioned in response to the user testing issue because this will add items to the translate vertical tab
Budapest Usability Testing Results
http://groups.drupal.org/node/271918

plach’s picture

Status: Needs work » Needs review
FileSize
36.56 KB

Rerolled

Status: Needs review » Needs work

The last submitted patch, et-metadata-1807800-47.patch, failed testing.

plach’s picture

Fixed a couple of issues with BC entities. Now tests should be passing again.

[Comment moved to #52]

Assigned: fago » Unassigned

The last submitted patch, et-metadata-1807800-49.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
38.09 KB
2.68 KB

Now for real :)

plach’s picture

Status: Needs work » Needs review
FileSize
37.79 KB
2.19 KB
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTranslationUITest.php
@@ -128,7 +126,7 @@ protected function assertPublishedStatus() {
-  function testTranslateLinkCommentAdminPage() {
+  function atestTranslateLinkCommentAdminPage() {

Reverted debug leftover. The interdiff is against #49.

Status: Needs review » Needs work

The last submitted patch, et-metadata-1807800-52.patch, failed testing.

plach’s picture

#1751606: Move published status checkbox next to "Save" went in meanwhile...

It's not clear to me whether according to #41 we can get this patch back to RTBC or we should refactor it. Personally I'd go with this as-is and refactor it once the needed API is in place. I'm afraid that if we wait for a complete API to be there, this won't be ready by the end of the feature completion phase.

plach’s picture

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTranslationUITest.php
@@ -60,23 +60,49 @@ function getTranslatorPermissions() {
+      // (Un)publish the node trnslations and check that the translation

Typo to be fixed on the next reroll.

plach’s picture

plach’s picture

Status: Needs work » Needs review

There was an IRC meeting today where we decided how to proceed with #1810370: Entity Translation API improvements. With the current proposal entity translation metadata would be regular multilingual entity fields living on the canonical data table.

In this scenario entities (node!) could alter their field definitions to skip any duplicate translation metadata. This way we'd avoid the confusion this issue has been generating so far.

That said I respectfully ask to mark this back to RTBC and commit it as-is. We will remove duplicate properties as soon as the NG conversion is over and we can proceed with the issue above.

YesCT’s picture

Status: Needs review » Needs work
Issue tags: +translatable fields, +API clean-up, +D8MI, +sprint, +language-content, +feature freeze, +budapest2012

The last submitted patch, et-metadata-1807800-56.patch, failed testing.

plach’s picture

FileSize
38.33 KB

Rerolled

Status: Needs review » Needs work
Issue tags: -translatable fields, -API clean-up, -D8MI, -sprint, -language-content, -feature freeze, -budapest2012

The last submitted patch, et-metadata-1807800-60.patch, failed testing.

plach’s picture

Status: Needs work » Needs review

#60: et-metadata-1807800-60.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +translatable fields, +API clean-up, +D8MI, +sprint, +language-content, +feature freeze, +budapest2012

The last submitted patch, et-metadata-1807800-60.patch, failed testing.

plach’s picture

Status: Needs work » Needs review

The bot failure is caused by a core bug which is being exposed by the new tests introduced here. We need #1899994-35: Disentangle language initialization from path resolution to be committed to make tests pass again.

plach’s picture

FileSize
38.34 KB

#1899994: Disentangle language initialization from path resolution went it, here is a reroll. Spoke with @catch in IRC and he said he might be ok with #57. Can we get this back to RTBC so he can have the final call?

Gábor Hojtsy’s picture

Assigned: Unassigned » catch
Status: Needs review » Reviewed & tested by the community

I think people above agree that this is an interim step and would be refactored later if possible. Otherwise the code looks as good as it can get AFAIS.

YesCT’s picture

#65: et-metadata-1807800-65.patch queued for re-testing.

catch’s picture

Title: Add status and authoring information as generic entity translation metadata » Change notice: Add status and authoring information as generic entity translation metadata
Category: feature » task
Status: Reviewed & tested by the community » Active

OK I went ahead and committed this. Tagging with revisit-before-release so we can check if things are back by the time that's done.

There's no new indexes added with the new columns. I can see there's no new queries added either except writing to the table but do we have an idea what queries are likely to be run and if so could some generic indexes that'd cover those be added?

YesCT’s picture

Issue tags: +revisit before beta

Revisit tag did not stick. Trying again.

plach’s picture

Title: Change notice: Add status and authoring information as generic entity translation metadata » Add status and authoring information as generic entity translation metadata
Status: Active » Fixed

Here is the change notice: http://drupal.org/node/1916780.

plach’s picture

Issue tags: -sprint

Removing from sprint, thank you all!

plach’s picture

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

clarify that more reviews are needed

catch’s picture

Issue tags: -revisit before beta

Untagging this, the entity schema generation has tidied all of this up.