Integration with ET and Title module

So, the ET integration (and its problem) aside, let's concentrate to Title integration in this issue. Patch looks great and it works. It really love to see that we could have some tests for it (with or without the locale module).

I just tested the Title module integration and it works good. Although when replacing the title field the original content does not get copied into the new title_field.

And I can confirm that it needs two clicks when saving a new translation. Any suggestions how to get rid of this @jcisio?

Tested with Scald 1.3, Drupal 7.34, Title 7.x-1.0-alpha7 and Entity Translation 7.x-1.0-beta4

Actually the dev version of the title module and ET module should be used.
Because both had a lot of changes since the last stable version, but mostly the title module where the last stable version was 2 years ago.

In the dev of title i get the following two errors with this patch:
Warning: Invalid argument supplied for foreach() in title_field_sync_get() (line 492 of /sites/all/modules/contrib/title/title.module).
Warning: Illegal string offset 'title' in title_entity_label() (line 130 of /sites/all/modules/contrib/title/title.module).

Here is a new patch for the title module dev version.

The functions return value has changed slightly, however so far i cant create a translation it goes to a bad request. So remains needs work.

Here is another patch that solves the broken translation add/edit links.

Because the following edit path will not work:
'edit path' => 'atom/%scald_atom/edit/%ctools_js',

As we cannot manage translations in the ctools modal anyways. And on the atom page tabs, the %ctools_js placeholder will never be solved.

Its still not ready, i will need to do some more tests on a fresh site.

So far it works fine and It seems i dont have issue number 1 where i would need to double save when adding a translation.

So far the problems are :
1. Cant change the title
2. Deleting translation does not work.

As far as I can see the issue with 2. is that the entity form does not have a delete button, and so entity translation cannot override the button to be the translation delete button.

The patch attached will also solve the translation deletion issue, by adding a delete translation button.

This also required to implement our own handler extending the default handler.

Now the only thing left to fix is to be able to change the title. Likely an error somewhere in the code at sync.

+1 issue i found is that
if 'edit path' => 'atom/%scald_atom/edit/%ctools_js',
Then the ctools edit pages works
if its the new value 'edit path' => 'atom/%scald_atom/edit',
then the links on the translate tab works.

So will need to find some solution to make both work.

Maybe with path schemes but not sure yet how to set that up.

The attached patch works better.
It fixes the issue with the %ctools_js, although not sure if it can be considered as a clean solution.

Also fixes the issue with the title not updating.

However the title field still seems to act up when there is no translation is added, and the user changes the default language value while changing the title.

Putting needs review to trigger the bots.

The other issue the patch has, is that when creating new atoms, the current code prefills the title, however it wont prefill the new title field.

To fix the prefill i added the following form alter in scald.module

/*
 * Implements hook_form_FORM_ID_alter().
 * In case the title module is used the title field has to be populated from the title
 * attribute when the user comes from the add step.
 */
function scald_form_scald_atom_add_form_options_alter(&$form, &$form_state, $form_id) {
  if (module_exists('title') && module_exists('entity_translation')) {
    $scald = $form_state['scald'];
    if (isset($scald['type'])) {
      // Setting default values for each created atom
      foreach($form as $key => $data) {
        if (strpos($key, 'atom') === 0) {
          if (!empty($form[$key]['title']['#default_value'])) {
            $langcode = $form['language']['#value'];
            $form[$key]['title_field'][$langcode][0]['value']['#default_value'] = $form[$key]['title']['#default_value'];
          }
        }
      }
    }
  }
}

And the following addition in the ScaldAtom construct after the values array definition

    if (module_exists('entity_translation')) {
      unset($values['language']);
      $handler = entity_translation_get_handler('scald_atom',(object) $values);
      $langcode = $handler->getLanguage();
      $values['language'] = $langcode;
    }

Unfortunately even though this seems right and the atom then correctly gets created by the default language specified in entity translation settings, the atom created this way is not possible to translate :( Because when i try to add a new translation it will just overwrite the original language version.

Okey the bug is not related to the code in #16

It is related to the fix in #13 where i replaced the %ctools_js out from the getEditPath in the handler.
Apparently if %ctools_js is in the entity info originally and since getEditPath does not run in all cases, then adding a new translation will not work, cause ET cannot attach itself to the edit page...

No idea why is it soo difficult to make it work.

Maybe it has something to do with the entity not being entity api based.

I think I'm postponing this until after 1.4, as i see no point including title integration when the ET integration is such a mess...

Fixed ET generated url and attached #16 code.

I'll review this patch.

I've removed the most annoying customization from Title (the sync set/get part). With our workaround sync_set, we lost backward compatibility e.g. $atom = scald_atom_load(1);$atom->title = 'Test title';scald_atom_save($atom); won't work (thanks to @DeFr). Now everything is OK in my test. Plupload integration still works, too.

I'm waiting for another round of feedback and then commit this patch.

I will have a look tomorrow.

I've read the patch again and just found that in scald_form_scald_atom_add_form_options_alter() we need to adjust the check in the same way as in title_field_replacement_info() of the last patch. module_exists('title') is not enough and title_field is hardcoded which is not always correct. I can't test right now so I'm not making another patch until this weekend.

It seems to me that even the title_field_replacement_info() is not enough. Because it is possible that I enabled the title replacement for a specific bundle only, like image, but did not enable it for videos lets say.

Something like this should solve those issues.

That's exactly it! I've rewrite the if condition because with the helper function from Title, we can write it shorter. Committed and pushed. Thanks all for this collaborative work!

The interdiff for reference.